Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem OSX(Catalina) plugin and NodeJS12+ #1981

Closed
oktapodia opened this issue Dec 1, 2019 · 14 comments
Closed

Problem OSX(Catalina) plugin and NodeJS12+ #1981

oktapodia opened this issue Dec 1, 2019 · 14 comments

Comments

@oktapodia
Copy link

oktapodia commented Dec 1, 2019

I have a problem when using a C++ native module on Nodejs 12 and above, the function always returns 0 instead of the expected value

  • Node Version: node 12.13.1 and npm 6.12.1
  • Platform: OSX Catalina (I followed the tutorial and everything looks well configured)
  • Compiler: Apple clang version 11.0.0 (clang-1100.0.33.12)
  • packages version:
    • node-addon-api: *
    • node-gyp: ^6.0.1
Binding.gyp:
{
  'targets': [{
    'target_name': 'testlib',
      'cflags!': [ '-fno-exceptions' ],
      'cflags_cc!': [ '-fno-exceptions' ],
      'xcode_settings': {
        'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
        'CLANG_CXX_LIBRARY': 'libc++',
        'MACOSX_DEPLOYMENT_TARGET': '10.7',
      },
    'include_dirs': [
      '<!@(node -p "require(\'node-addon-api\').include")',
    ],
    'dependencies': ["<!(node -p \"require('node-addon-api').gyp\")"],

    'cflags': [
      '-Wall',
      '-Wparentheses',
      '-Winline',
      '-Wbad-function-cast',
      '-Wdisabled-optimization'
    ],

    'conditions': [
      ['OS == "mac"', {
          'cflags+': ['-fvisibility=hidden'],
          'xcode_settings': {
            'GCC_SYMBOLS_PRIVATE_EXTERN': 'YES', # -fvisibility=hidden
          },
        'include_dirs': [
          '<!@(node -p "require(\'node-addon-api\').include")',
          'System/Library/Frameworks/CoreFoundation.Framework/Headers',
          'System/Library/Frameworks/Carbon.Framework/Headers',
          'System/Library/Frameworks/ApplicationServices.framework/Headers',
          'System/Library/Frameworks/OpenGL.framework/Headers',
        ],
        'link_settings': {
          'libraries': [
            '-framework Carbon',
            '-framework CoreFoundation',
            '-framework ApplicationServices',
            '-framework OpenGL'
          ]
        }
      }],
    ],

    'sources': [
        'src/main.cpp',
        'src/main/screen.c',
    ]
  }]
}
screen.c:
#if defined(IS_MACOSX)
	#include <ApplicationServices/ApplicationServices.h>
#endif

MMSize getDisplaySize(void)
{
#if defined(IS_MACOSX)
	CGDirectDisplayID displayID = CGMainDisplayID();
	return MMSizeMake(CGDisplayPixelsWide(displayID),
	                  CGDisplayPixelsHigh(displayID));
#endif
}
main.cpp:
#include <napi.h>
#include "screen.h"

Napi::Object screen::getScreenSize(const Napi::CallbackInfo& info) {
    Napi::Env env = info.Env();

    MMSize displaySize = getDisplaySize();
    Napi::Object obj = Napi::Object::New(env);

    obj.Set("width", displaySize.width);
    obj.Set("height", displaySize.height);

    return obj;
}

Napi::Object InitAll(Napi::Env env, Napi::Object exports) {
    screen::Init(env, exports);

    return exports;
}

NODE_API_MODULE(NODE_GYP_MODULE_NAME, InitAll)

Output 11 and below:

{ width: 1920, height: 1200 }

Output Nodejs 12+:

{ width: 0, height: 0 }

I've added more information about the code, this might be helpful.

I don't really know if it is a problem about node-gyp or napi

Any ideas?

PS: It is working well on linux with a X11 code

Regards

@rvagg
Copy link
Member

rvagg commented Dec 2, 2019

I don't think we have have enough information to be helpful here. Your screen.c isn't addon code, is it printing those values in c++ or is that what's returning to JavaScript somewhere in main.cpp?

@oktapodia
Copy link
Author

Thank you for your comment @rvagg I've updated my main post

@cclauss
Copy link
Contributor

cclauss commented Dec 2, 2019

Would it be possible to put this code into a Travis CI test or a GitHub Actions test?

@rvagg
Copy link
Member

rvagg commented Dec 2, 2019

OK, this is pretty interesting, can confirm that something is screwy. My minimal repro:

package.json:

{
  "name": "addon",
  "gypfile": true,
  "dependencies": {
    "node-addon-api": "~2.0.0"
  }
}

binding.gyp:

{
  'targets': [{
    'target_name': 'testlib',
    'xcode_settings': {
      'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
    },
    'include_dirs': [
      '<!@(node -p "require(\'node-addon-api\').include")',
    ],
    'dependencies': ["<!(node -p \"require('node-addon-api').gyp\")"],
    'conditions': [
      ['OS == "mac"', {
        'include_dirs': [
          'System/Library/Frameworks/ApplicationServices.framework/Headers'
        ],
        'link_settings': {
          'libraries': [
            '-framework ApplicationServices',
          ]
        }
      }],
    ],
    'sources': [
      'main.cpp',
    ]
  }]
}

main.cpp:

#include <napi.h>
#include <ApplicationServices/ApplicationServices.h>

Napi::Object getScreenSize(const Napi::CallbackInfo& info) {
    Napi::Env env = info.Env();
    Napi::Object obj = Napi::Object::New(env);
    CGDirectDisplayID displayID = CGMainDisplayID();
    obj.Set("width", CGDisplayPixelsWide(displayID));
    obj.Set("height", CGDisplayPixelsHigh(displayID));
    fflush(stdout);
    return obj;
}

Napi::Object InitAll(Napi::Env env, Napi::Object exports) {
  return Napi::Function::New(env, getScreenSize);
}

NODE_API_MODULE(NODE_GYP_MODULE_NAME, InitAll)

Test with: node -pe "require('./build/Release/testlib.node')()"

Rebuild with node-gyp rebuild

On Node 10.x it prints out a proper resolution, on 13.x it prints out { width: 0, height: 0 }.

Minimal direct application to sanity check:

run.cpp

#include <stdio.h>
#include <ApplicationServices/ApplicationServices.h>

int main() {
    CGDirectDisplayID displayID = CGMainDisplayID();
    printf("{ width: %lu, height: %lu }\n", CGDisplayPixelsWide(displayID), CGDisplayPixelsHigh(displayID));
    fflush(stdout);
    return 0;
}

compile with: clang run.cpp -framework ApplicationServices -o run.

@nodejs/addon-api @nodejs/n-api anyone care to take a poke at this one? I'm not sure where to go. Related to how we build Node now, with the libnode setup perhaps? Or is there some flag we're setting now that is changing the build enough for this to be different? The only stand-out difference I can see in common.gypi is MACOSX_DEPLOYMENT_TARGET being 10.10 not, but the original report has it as 10.7, the same as we've had in 10.x and a few prior versions.

@oktapodia
Copy link
Author

oktapodia commented Dec 2, 2019

I was actually creating the repository but thank you @rvagg this is exactly the problem I have.

I just want to add that it is working fine on linux with node 12+ and the following code

	Display *display = XGetMainDisplay();
	const int screen = DefaultScreen(display);

	return MMSizeMake((size_t)DisplayWidth(display, screen),
	                  (size_t)DisplayHeight(display, screen));

@rvagg
Copy link
Member

rvagg commented Dec 2, 2019

Doing a V=1 node-gyp rebuild and watching the compile output with old and new Node, the only real differences are:

'-DV8_DEPRECATION_WARNINGS'
'-DV8_IMMINENT_DEPRECATION_WARNINGS'
'-DOPENSSL_THREADS'

And the -mmacosx-version-min=10.7 vs -mmacosx-version-min=10.10 change.

Then there's this: if I compile the addon with 13.x and run it against both 13.x and 10.x, it works with 10.x but doesn't with 13.x. So I guess that places the blame in Node itself, and/or how it's built:

$ node -v
v13.2.0
$ node -pe "require('./build/Release/testlib.node')()"
{ width: 0, height: 0 }
$ ./node-v10.17.0-darwin-x64/bin/node -v
v10.17.0
$ ./node-v10.17.0-darwin-x64/bin/node -pe "require('./build/Release/testlib.node')()"
{ width: 1440, height: 900 }

@oktapodia
Copy link
Author

@NickNaso Any thought on that one?

@gabrielschulhof
Copy link

Alas, I think this requires an OSX laptop, which I do not have 🙁

@oktapodia
Copy link
Author

Yes, it is :/ Maybe @romandev or @mhdawson have an OSX computer?

@NickNaso
Copy link
Member

NickNaso commented Dec 7, 2019

Hi everybody,
I'm travelling by I tried to investigate the problem. What I found is that the reported behaviour happens starting from the following versions of Node.js:

  • v12.13.1
  • v13.0.1

@cclauss
Copy link
Contributor

cclauss commented Dec 7, 2019

Travis CI and GitHub Actions both have Macs... Can we create a test that fails there?

@oktapodia
Copy link
Author

@cclauss Here's a reproducible example with the travis tests failing on node 12+

@legendecas
Copy link
Member

legendecas commented Dec 8, 2019

Confirmed issue related to libuv/libuv#2566.

It could be reproduced on Node.js v10 by setting the process.title before retrieving the window size.

@bnoordhuis
Copy link
Member

This was fixed a while ago in libuv (or fixed... it wasn't really a libuv issue to start with but that aside) so I'm going to close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants