Skip to content

node-addon-api fails to build against node 4.x #142

Closed
Closed
node-addon-api fails to build against node 4.x#142

Description

@gabrielschulhof
gabrielschulhof
opened on Sep 25, 2017
$ nvm use 4
$ npm test

Observe build failure.

Activity

  1. gabrielschulhof commented on Sep 25, 2017

    @gabrielschulhof
    gabrielschulhof
    on Sep 25, 2017
    ContributorAuthor

    I tweaked the node-test-node-addon-api job a bit:

    https://ci.nodejs.org/job/node-test-node-addon-api/1/ (nightly - good)
    vs.
    https://ci.nodejs.org/job/node-test-node-addon-api/2/ (v4 - bad)

  2. gabrielschulhof commented on Sep 25, 2017

    @gabrielschulhof
    gabrielschulhof
    on Sep 25, 2017
    ContributorAuthor

    v6 is also bad: https://ci.nodejs.org/job/node-test-node-addon-api/3/

  3. mhdawson commented on Sep 25, 2017

    @mhdawson
    mhdawson
    on Sep 25, 2017
    Member

    @gabrielschulhof can I assume you are investigating ? It is possible that its related to the async changes that were landed in master/head but I doubt have been backported to 4/6

  4. gabrielschulhof commented on Sep 25, 2017

    @gabrielschulhof
    gabrielschulhof
    on Sep 25, 2017
    ContributorAuthor

    Is this #144?

  5. gabrielschulhof commented on Sep 25, 2017

    @gabrielschulhof
    gabrielschulhof
    on Sep 25, 2017
    ContributorAuthor

    @mhdawson almost certainly, since it's precisely async that is tripping things up.

  6. gabrielschulhof commented on Sep 27, 2017

    @gabrielschulhof
    gabrielschulhof
    on Sep 27, 2017
    ContributorAuthor

    Actually, node-addon-api fails to build against nov 8.4.0 too. So AFAICT it's basically broken against any version of node other than the latest.

  7. gabrielschulhof commented on Sep 28, 2017

    @gabrielschulhof
    gabrielschulhof
    on Sep 28, 2017
    ContributorAuthor

    I'm starting to go through the errors, and I've noticed that we've introduced a call to v8::FunctionCallbackInfo's NewTarget() method. This method is not available in previous versions of V8, so can we fall back onto This()? Can NewTarget() be something other than This() when IsConstructCall() is true? @nodejs/addon-api

  8. bnoordhuis
    added a commit that references this issue on Sep 29, 2017

    test: check that this != new.target in addon

    b24110a

  9. bnoordhuis commented on Sep 29, 2017

    @bnoordhuis
    bnoordhuis
    on Sep 29, 2017
    Member

    Can NewTarget() be something other than This() when IsConstructCall() is true?

    Yes, see nodejs/node#15681. You could back-port nodejs/node#11652 to v4.x.

  10. BridgeAR
    added a commit that references this issue on Oct 2, 2017

    test: check that this != new.target in addon

    9e9c83e
  11. MylesBorins
    added 2 commits that reference this issue on Oct 3, 2017

    test: check that this != new.target in addon

    8c7c7ad

    test: check that this != new.target in addon

    a75f042
  12. addaleax
    added a commit that references this issue on Oct 4, 2017

    test: check that this != new.target in addon

    33c6df5
  13. MylesBorins
    added a commit that references this issue on Oct 11, 2017

    test: check that this != new.target in addon

    88f69d3

  14. gabrielschulhof commented on Oct 13, 2017

    @gabrielschulhof
    gabrielschulhof
    on Oct 13, 2017
    ContributorAuthor

    Hmmm ... on node 8.0.0 it's building our copy of node_api.cc and it's creating node-api.a, but then the linker resolves calls to node's own version, meaning the ABI is mismatched, running into segfaults as a result.

  15. 2 remaining items

  16. gabrielschulhof commented on Oct 13, 2017

    @gabrielschulhof
    gabrielschulhof
    on Oct 13, 2017
    ContributorAuthor

    @bnoordhuis
    nm -C
    ldd:

    	linux-vdso.so.1 (0x00007ffd43787000)
	libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f2078d42000)
	libm.so.6 => /lib64/libm.so.6 (0x00007f2078a2c000)
	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f2078815000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f20785f6000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f2078225000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f207932b000)

    V=1 npm test

    I think it's a library load time issue. The symbols are available both locally (via node-api.a) and from Node.js, and the dynamic linker is resolving the needed symbols using Node's symbols rather than those from node-api.a, which is statically linked into binding.node.

    Defining NAPI_EXTERN as __attribute__((visibility("hidden"))) causes the symbols in node-api.a to take precedence.

  17. gabrielschulhof commented on Oct 13, 2017

    @gabrielschulhof
    gabrielschulhof
    on Oct 13, 2017
    ContributorAuthor

    Wow! Another weird consequence. The Node.js async execute callback gets called, even though the async work item is created inside node-api.a.

  18. gabrielschulhof commented on Oct 13, 2017

    @gabrielschulhof
    gabrielschulhof
    on Oct 13, 2017
    ContributorAuthor

    That is, the version of uvimpl::Work::ExecuteCallback available inside node gets called, not the one available inside node-api.a.

  19. gabrielschulhof commented on Oct 13, 2017

    @gabrielschulhof
    gabrielschulhof
    on Oct 13, 2017
    ContributorAuthor

    I have to qualify uvimpl::Work::ExecuteCallback and uvimpl::Work::CompleteCallback with NAPI_EXTERN in order for them to get resolved correctly.

  20. bnoordhuis commented on Oct 16, 2017

    @bnoordhuis
    bnoordhuis
    on Oct 16, 2017
    Member

    I also left a comment on nodejs/node#16234 but if you look at the output from nm, there are a lot of symbols in there that probably have no business being weak:

    ...
000000000002bafb W napi_value__* Napi::details::WrapCallback<Napi::RegisterModule(napi_env__*, napi_value__*, Napi::Object (*)(Napi::Env, Napi::Object))::{lambda()#1}>(Napi::RegisterModule(napi_env__*, napi_value__*, Napi::Object (*)(Napi::Env, Napi::Object))::{lambda()#1})
0000000000037078 W Napi::details::AccessorCallbackData<Napi::Value (*)(Napi::CallbackInfo const&), void (*)(Napi::CallbackInfo const&)>::GetterWrapper(napi_env__*, napi_callback_info__*)
0000000000037159 W Napi::details::AccessorCallbackData<Napi::Value (*)(Napi::CallbackInfo const&), void (*)(Napi::CallbackInfo const&)>::SetterWrapper(napi_env__*, napi_callback_info__*)
000000000002f638 W Napi::External<int>::New(napi_env__*, int*)
000000000002f302 t Napi::External<int> Napi::External<int>::New<(anonymous namespace)::CreateExternalWithFinalize(Napi::CallbackInfo const&)::{lambda(Napi::Env, int*)#1}>(napi_env__*, int*, (anonymous namespace)::CreateExternalWithFinalize(Napi::CallbackInfo const&)::{lambda(Napi::Env, int*)#1})
000000000002f3e4 t Napi::External<int> Napi::External<int>::New<(anonymous namespace)::CreateExternalWithFinalizeHint(Napi::CallbackInfo const&)::{lambda(Napi::Env, int*, char*)#1}, char>(napi_env__*, int*, (anonymous namespace)::CreateExternalWithFinalizeHint(Napi::CallbackInfo const&)::{lambda(Napi::Env, int*, char*)#1}, char*)
000000000002f7b8 W Napi::External<int>::External(napi_env__*, napi_value__*)
...
  21. gabrielschulhof
    added 2 commits that reference this issue on Oct 16, 2017

    Fix test errors

    27e2f9b

    Fix test errors

    005cc1d
  22. gabrielschulhof
    closed this as completedin 005cc1don Oct 17, 2017
  23. gabrielschulhof
    added a commit that references this issue on Oct 17, 2017

    Fix test errors

    739b51e
  24. MylesBorins
    added a commit that references this issue on Oct 17, 2017

    test: check that this != new.target in addon

    313554a
  25. MylesBorins
    added a commit that references this issue on Oct 25, 2017

    test: check that this != new.target in addon

    e3ea2a4
  26. kevindavies8
    added a commit that references this issue on Aug 24, 2022

    Fix test errors

    d46a1b7
  27. Marlyfleitas
    added a commit that references this issue on Aug 26, 2022

    Fix test errors

    d452adc
  28. wroy7860
    added a commit that references this issue on Sep 19, 2022

    Fix test errors

    2769452
  29. johnfrench3
    added a commit that references this issue on Aug 11, 2023

    Fix test errors

    ea1f05c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions