src: fix build on CentOS

[Trott]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

Description of change

85af1a6 was added (squashed into another commit) without running through CI. Unfortunately, it breaks some of the CI builds, notably on three of the CentOS setups.

Undoing that one small change to get builds green again.

[Trott]

CI: https://ci.nodejs.org/job/node-test-pull-request/3402/

[Trott]

/cc @jbergstroem @danbev @addaleax @trevnorris @bnoordhuis

[Fishrock123]

rubber-stamp lgtm if CI passes

[jbergstroem]

Thanks for this. Bots pass again -- LGTM.

[bnoordhuis]

LGTM. ci.nodejs.org 502s for me so I don't know what the actual build error was.

[jbergstroem]

Jenkins just had a OOM. Sometimes 28G ram just ain't enough -- should be back now, but it seems to be running a git gc over 12 cores so give it some breathing room!

[Trott]

Hmmm...old CI is now 404, and I'm not sure if anyone saw it finish or not, so let's try again:

CI: https://ci.nodejs.org/job/node-test-pull-request/3406/

[addaleax]

LGTM if CI is green

[Trott]

If CI is green, I'm going to land this right away to fix the CI builds. If anyone objects, please lodge your objection ASAP here. Thanks!

[Trott]

ubuntu1604-arm64 has been failing builds for two or three days now for apparently different reasons so I'm going to ignore that one. Otherwise, CI is green so far...

[Trott]

One Raspberry Pi buildbot failure too, but that's it. Landing...

[Trott]

Landed in 809aabc. Thanks, everyone!

[danbev]

Sorry about causing this and creating the extra work for you, and thanks for fixing it.

I went looking at the failing CI builds but was not able to find much information, which might just be that I don't have the correct permission or something. What I wanted was to see any logs of the failure to help me understand the mistake I made to avoid this in the future.

What I had originally done was added an empty destructor:

    ~ConnectionWrap() {
    }

I later changed this to be an explicitly defaulted destructor generated by the compiler:

    ~ConnectionWrap() = default; 

My understanding/assumption was that these two would be equivalent.

To give some context, this is what the inheritance tree for ConnectionWrap looks like, with regards to destructors:

    class ConnectionWrap : public StreamWrap
      protected:
        ~ConnectionWrap() {}

    class StreamWrap : public HandleWrap, public StreamBase
      protected:
        ~StreamWrap() { }

    class HandleWrap : public AsyncWrap
      protected:
        ~HandleWrap() override;

    class AsyncWrap : public BaseObject
      public:
        inline virtual ~AsyncWrap();

    class BaseObject
      public:
        inline virtual ~BaseObject();

    class StreamBase : public StreamResource
      public:
        virtual ~StreamBase() = default;

    class StreamResource
      public:
        virtual ~StreamResource() = default;

If anyone can explain what I'm missing here it would be much appreciated.

[addaleax]

The CI is a bit hard to navigate, you can find an output of a “real” compilation e.g. here. It shows internal compiler errors, so it’s definitely not your fault, and rather mine than anyone else’s for not running CI again with the last commit included before landing.

[danbev]

@addaleax Thanks for the link to the compilation error.

[danbev]

From looking at the error:

In file included from ../src/pipe_wrap.h:7:0,
                 from ../src/pipe_wrap.cc:1:
../src/connection_wrap.h:26:3: internal compiler error: in use_thunk, at cp/method.c:338
   ~ConnectionWrap() = default;
   ^
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-4.8/README.Bugs> for instructions.
Preprocessed source stored into /tmp/ccvbqQz3.out file, please attach this to your bugreport.
ERROR: Cannot create report: [Errno 17] File exists: '/var/crash/_usr_lib_gcc_x86_64-linux-gnu_4.8_cc1plus.1000.crash'
make[2]: *** [/home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1204-64/out/Release/obj.target/node/src/pipe_wrap.o] Error 1

it looks like GCC (G++) 4.8 is being used. The reason for asking is I did a search and found a few indications that this might be a bug in the compiler. This is reported as sovled in 4.8.3 which is also why I'm curious about the compiler version.

I'm don't mean to imply that the compiler version should be updated, just want to understand the issue here.

[bnoordhuis]

I believe the centos machines use devtoolset-2, which comes with g++ 4.8.2.

[danbev]

@bnoordhuis Ah great, thanks for that.

[jbergstroem]

Just confirming: we're using devtoolset-2.