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

Add NODE_RTLD_GLOBAL to control library loading #4105

Closed
wants to merge 1 commit into from
Closed

Conversation

Y--
Copy link

@Y-- Y-- commented Dec 1, 2015

Setting the environment variable NODE_RTLD_GLOBAL to true set RTLD_GLOBAL flag when loading module.

This pull request is motivated by the fact that dlopen does not have the same behavior on MacOS X and Linux (cf. dlopen man page) :

  • On MacOS X / BSD :
If neither RTLD_GLOBAL nor RTLD_LOCAL is specified, the default is RTLD_GLOBAL.
  • On Linux :
  RTLD_LOCAL
    This is the converse of RTLD_GLOBAL, and the default if neither flag is specified.

I was not sure what name would be the best to use for this purpose, so this is a first suggestion, but I'd be happy to change them. Also, I've implemented that as an opt-in mode, with as few changes as I could, but I'd be also happy to extend the idea and provide more flexibility.

Note : I'm no Windows expert, so I was not sure how what to do for this implementation.

Many thanks in advance for considering this patch.

`NODE_RTLD_GLOBAL=true` set `RTLD_GLOBAL` flag when loading module
@cjihrig
Copy link
Contributor

cjihrig commented Dec 1, 2015

If your PR requires changes to libuv (files in deps/uv), could you open a PR there first?

@Y--
Copy link
Author

Y-- commented Dec 1, 2015

Sure, I will do that right now. Thanks

@Fishrock123 Fishrock123 added fs Issues and PRs related to the fs subsystem / file system. module Issues and PRs related to the module subsystem. libuv Issues and PRs related to the libuv dependency or the uv binding. labels Dec 1, 2015
@cjihrig
Copy link
Contributor

cjihrig commented Dec 1, 2015

Thanks. Closing this for now.

@cjihrig cjihrig closed this Dec 1, 2015
@Y--
Copy link
Author

Y-- commented Dec 1, 2015

I've created a PR in libuv there : libuv/libuv#635

@ezequielgarcia ezequielgarcia mentioned this pull request May 2, 2017
3 tasks
refack pushed a commit that referenced this pull request Sep 8, 2017
* add constants for dlopen flags, which are needed
  for dlopen's flag passing.

* introduce an optional parameter for process.dlopen(),
  allowing to pass dlopen flags (using values from os.constants.dlopen).

If no flags are passed, the default behavior is to load the library
with RTLD_LAZY (perform lazy binding) and RTLD_LOCAL (symbols are
available only locally).

PR-URL: #12794
Refs: #4105
Refs: libuv/libuv#1331
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
addaleax pushed a commit to addaleax/node that referenced this pull request Sep 13, 2017
* add constants for dlopen flags, which are needed
  for dlopen's flag passing.

* introduce an optional parameter for process.dlopen(),
  allowing to pass dlopen flags (using values from os.constants.dlopen).

If no flags are passed, the default behavior is to load the library
with RTLD_LAZY (perform lazy binding) and RTLD_LOCAL (symbols are
available only locally).

PR-URL: nodejs#12794
Refs: nodejs#4105
Refs: libuv/libuv#1331
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants