Add process.dlopenFlags #12794

ezequielgarcia · 2017-05-02T17:29:48Z

This PR is another attempt at support dlopen flags. A previous try was made in #4105, but it never got accepted.

In particular, it's common to require a node addon to be loaded using dlopen(flag=RTLD_GLOBAL). For instance, this is required to implement a libao binding, due to libao's plugins being shared objects that need symbols exposed in a common shared object (see here).

It's interesting to mention that this feature is being requested since at least 2010:

nodejs/node-v0.x-archive#436
https://groups.google.com/forum/#!topic/nodejs/L2wNb_azzyU

I really hope we can tackle it this time, so please provide any feedback to make this feature acceptable.

The code is a little bit ugly, open-coding the dlopen call instead of abstracting it behind libuv. My initial attempt was through libuv, but it got rejected: libuv/libuv#1331.

This is inspired by Python sys.{get,set}dlopenflags. Python docs here and here.

Checklist

make -j4 test (UNIX) passes
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

src, process

mscdex · 2017-05-02T17:32:49Z

doc/api/os.md

+### dlopen Constants
+
+The following signal constants are exported by `os.constants.dlopen`.
+See `man 3 dlopen` for detailed informatio.


s/informatio/information/

Also, it might be better to use the automatic man page linking by formatting the name according to how it's done elsewhere (e.g. in the fs docs).

mscdex · 2017-05-02T17:34:20Z

doc/api/os.md

+    <th>Constant</th>
+  </tr>
+  <tr>
+    <td><code>RTLD_LAZY</code></td>


Missing descriptions for these constants?

I thought the descriptions were too detailed and complex, and was expecting the reader could refer to the manual instead? It's a 1:1 mapping of the dlopen flags, so there's not any secret sauce.

If the idea is to refer the user to a manual, then having these as links to that manual would be ideal :-) ... still, a very brief one sentence description would be preferred (I know we're fairly inconsistent with that in the docs and we need to get better at it)

@mscdex @jasnell I've added a line with some description. I agree it looks better now.

mscdex · 2017-05-02T17:36:04Z

src/node_constants.cc

@@ -1287,6 +1321,10 @@ void DefineConstants(v8::Isolate* isolate, Local<Object> target) {
  CHECK(zlib_constants->SetPrototype(env->context(),
                                     Null(env->isolate())).FromJust());

+  Local<Object> dlopen_constants = Object::New(isolate);
+  CHECK(dlopen_constants->SetPrototype(env->context(),
+                                     Null(env->isolate())).FromJust());


This line should be indented to align with the first argument to SetPrototype().

mscdex · 2017-05-02T17:37:44Z

src/node.cc

+
+#ifdef __POSIX__
+  // If no flag is passed, fallback to default
+  if (args.Length() < 3 || args[2]->IsUndefined() ||


What about falling back to the old behavior if it's simply not an integer instead of just undefined or null? That might make more sense for nonsensical values that could be passed (e.g. object, boolean, etc.).

Sure, that works.

On a second thought:

Shouldn't we better check the type and fail if it's not an integer? The only accepted values/types would be integers, boolean, null and undefined.

dlopen can't be called with a zero flag, so I was treating the 0 as if the user requested passed null, allowing to fallback to default.

I suppose that's fine as well.

addaleax · 2017-05-02T17:32:14Z

doc/api/process.md

@@ -683,6 +683,22 @@ process.on('warning', (warning) => {

 If `warning` is passed as an `Error` object, the `options` argument is ignored.

+## process.dlopenFlags
+<!-- YAML
+added: v8.0.0


Please use added: REPLACEME :)

addaleax · 2017-05-02T17:38:57Z

src/node.cc

+#ifdef __POSIX__
+  // If no flag is passed, fallback to default
+  if (args.Length() < 3 || args[2]->IsUndefined() ||
+      args[2]->IsNull() || args[2]->IntegerValue() == 0) {


IntegerValue() without arguments is deprecated, so it would be good if you could either use the overload that returns a Maybe<int64_t> and pass along the error if one occurs, or alternatively throw a type error if args[2]->IsInt32() fails and then use args[2].As<Int32>()->Value() to get the flags

how about something like:

int flags = 0; if (args.Length() > 2) { if (!args[2]->IsInt32() && !args[2]->IsUndefined()) { return env->ThrowTypeError("dlopen flags must be undefined or integer."); } if (args[2]->IsInt32()) flags = args[2]->Int32Value(); } if (!flags) { // If no flag is passed, fallback to default is_dlopen_error = uv_dlopen(*filename, &lib); } else { [the dlopen with flags]

jasnell

Question: should this be an API or a command line flag? What are the risks involved with different modules requiring different flags? Perhaps this is something that should be exposed off the module specific require instead (e.g. require.setdlopenFlags())? Just trying to think through the various ramifications.

jasnell · 2017-05-02T17:54:08Z

doc/api/process.md

+process.dlopenFlags = os.constants.dlopen.RTLD_NOW | os.constants.dlopen.RTLD_GLOBAL;
+var foo = require('foo');
+process.dlopenFlags = null;
+```


Not sure if a property is the right approach. I'd prefer a function (e.g. setdlopenFlags()/getdlopenFlags() that would include type checking on the value passed.

I'm fine either way. Whatever you guys decide it works better.

ezequielgarcia · 2017-05-02T18:30:00Z

I don't how doable a require.setdlopenFlags() would be, but it might be the cleanest option.

About the risks of passing different flags to different modules, I don't think this is a concern, if used properly. We'd want to pass a flag to load a specific addon and then go back to default, like this:

process.dlopenFlags = os.constants.dlopen.RTLD_NOW | os.constants.dlopen.RTLD_GLOBAL;
var binding = require('bindings')('binding');
process.dlopenFlags = null;

Shared libraries should have properly named symbols, to avoid namespace collision. In any case, we are just providing a mechanism, it's up to developers to avoid shooting themselves. Not sure this answers your question.

sam-github · 2017-05-04T09:51:48Z

Question: should this be an API or a command line flag? What are the risks involved with different modules requiring different flags? Perhaps this is something that should be exposed off the module specific require instead (e.g. require.setdlopenFlags())? Just trying to think through the various ramifications.

^--- I had same questions as @jasnell from above. This seems it should be localized to a specific addon load, no?

ezequielgarcia · 2017-05-04T15:37:04Z

@sam-github @jasnell Yes, it should be localized to one addon. However, it sounds that would require adding dlopenflags awareness to the bindings module. Any other idea?

gireeshpunathil · 2017-05-07T07:43:55Z

Java virtual machines were plagued by namespace collision introduced through RTLD_GLOBAL - for example, an incoming symbol through dlopen could collide with an existing symbol in the already loaded images, and load order and other dependancy constraints come into play. To make things worse, JVM could be launched as a shared library from an existing process with already loaded libraries and symbols, and then the load order reverses, and the behavior too.

On the other hand, I recognize the use case, and the need to be able to expose loaded symbols outside of the library confinement. The current approach of platform default not only is restrictive, but also is platform dependant.

In my opinion, (i) the option to load addons with RTLD_GLOBAL should be localized on a per module basis to reduce side effects(ii) the functionality should be made platform neutral to reduce behavioral inconsistencies.

ezequielgarcia · 2017-05-12T00:12:01Z

Hey guys,

First of all, thanks for the great feedback. I'm having lots of fun hacking node!

Just updated the branch, exploring the require(..., opts) route, as per the discussion in #12848. Hopefully, I've addresses all the feedback received around API usage and coding style.

jasnell · 2017-05-12T14:54:21Z

lib/module.js

@@ -447,6 +447,10 @@ Module._load = function(request, parent, isMain) {
  }

  var module = new Module(filename, parent);
+  if (opts !== undefined && opts.dlopenFlags) {


perhaps if (opts !== null && typeof opts === 'object' && opts.dlopenFlags) to make the check more precise.

as it is here, `require('foo', null) would throw an error

note that 'object' type is checked below. I was expecting Module._load to be called strictly with either 'undefined' or 'object'. Being internal, I thought I could expect that. let me know if that sounds ok.

Unfortunately that is not a safe assumption. assert should only be used when we know with absolute certainty that the value being tested comes from Node.js itself. In this case, the value comes from the user. Further, this test will fail if opts === null since typeof null === 'object' and null !== undefined

jasnell · 2017-05-12T14:58:29Z

lib/module.js

@@ -447,6 +447,10 @@ Module._load = function(request, parent, isMain) {
  }

  var module = new Module(filename, parent);
+  if (opts !== undefined && opts.dlopenFlags) {
+    assert(util.isNumber(opts.dlopenFlags), 'dlopenFlags must be a number');


This should not be an assert since this is checking a user provided value.
Instead, you'll want something like:

if (!util.isNumber(opts.dlopenFlags)) { const errors = require('internal/errors'); throw new errors.TypeError('ERR_INVALID_OPT_VALUE', 'dlopenFlags', opts.dlopenFlags); }

jasnell · 2017-05-12T14:59:07Z

lib/module.js

@@ -447,6 +447,10 @@ Module._load = function(request, parent, isMain) {
  }

  var module = new Module(filename, parent);
+  if (opts !== undefined && opts.dlopenFlags) {
+    assert(util.isNumber(opts.dlopenFlags), 'dlopenFlags must be a number');
+    module._dlopenFlags = opts.dlopenFlags;


Consider using a non-enumerable Symbol here instead of a new _ prefixed property.

e.g.

const kOpenFlags = Symbol('node-dlopenflags'); // ... Object.define(module, kOpenFlags, { enumerable: false, value: opts.dlopenFlags });

jasnell · 2017-05-12T15:01:44Z

lib/module.js

  assert(path, 'missing path');
  assert(typeof path === 'string', 'path must be a string');
-  return Module._load(path, this, /* isMain */ false);
+  assert(opts === undefined || typeof opts === 'object', 'opts must be an object');


As above, this should not be an assert since it is testing user input. A TypeError should be used instead.

@jasnell Makes sense. Should the assert on the path argument be changed to exceptions too?

Also, I'm really wondering if checking the argument here makes any sense at all. The type check at _load might be just enough.

jasnell

Left some comments. Getting closer. Still need some more @nodejs/ctc folks to weigh in on this direction tho.

gireeshpunathil

I have a general documentation comment: Given that we are widening the symbol scope beyond the library being loaded and thereby opening up potential collisions, it warrants a 'specification statement' in the doc so that we reduce production issues as well as friction with module owners. For example, stereotypical words like cb(though used in more JS than in C++), read(), push() etc. can collide with names in node. On the other hand, module owners can claim that their name(s) pre-date node symbols, and node should rename its symbol to resolve the conflict.

If you agree, my attempt to the spec thus:

"The module beling loaded should make sure that it does not collide with existing symbols which are loaded into the application and virtual machine space, and any issue arises thereon should be resolved by renaming symbols in the library which caused it."

I don't have any comments on the code - with the suggestions from @jasnell, it looks fine to me.

ezequielgarcia · 2017-05-22T16:11:39Z

@jasnell @addaleax @mscdex Just updated the PR addressing James' feedback.

mscdex · 2017-05-22T16:32:03Z

doc/api/modules.md

+
+<!-- type=misc -->
+
+require() supports an optional parameter than can be used to pass


s/request()/`require()`/

mscdex · 2017-05-22T16:32:49Z

doc/api/modules.md

+<!-- type=misc -->
+
+require() supports an optional parameter than can be used to pass
+an object containing modifiers to the require operation.


s/require operation/module loader/ ?

mscdex · 2017-05-22T16:47:40Z

doc/api/modules.md

+an object containing modifiers to the require operation.
+
+Currently, the only property supported is `dlopenFlags`, which
+can be used to pass flags to the `dlopen` call that loads


This sentence sounds a bit odd to me. Perhaps reword/restructure the whole section to something like:

Valid properties:

dlopenFlags {integer} dlopen(3) flags to be used when loading a Node.js addon. See the documentation for [os.constants.dlopen][] for valid flags. Example:

var dlopen = require('os').constants.dlopen; var foo = require('./foo.node', { dlopenFlags: dlopen.RTLD_NOW | dlopen.RTLD_GLOBAL });

bnoordhuis

I have several misgivings about this pull request, the biggest being that I think it's a bad idea to extend require() for an esoteric use case that require.resolve() + process.dlopen() would also serve.

I can live with extending process.dlopen() but I'm -1 on touching require().

bnoordhuis · 2017-05-22T17:17:44Z

doc/api/os.md

+  </tr>
+  <tr>
+    <td><code>RTLD_DEEPBIND</code></td>
+    <td>Make a self-contained library use its own symbols in preference to symbols from previously loaded libraries.</td>


Long lines in this file, please wrap at 80 columns.

bnoordhuis · 2017-05-22T17:18:09Z

doc/api/modules.md

+
+```js
+var os = require('os');
+var foo = require('./foo', {dlopenFlags: os.constants.dlopen.RTLD_NOW | os.constants.dlopen.RTLD_GLOBAL})


Long line, please wrap at 80 columns.

bnoordhuis · 2017-05-22T17:22:57Z

doc/api/os.md

@@ -1160,6 +1160,46 @@ The following error codes are specific to the Windows operating system:
  </tr>
 </table>

+### dlopen Constants
+
+The following signal constants are exported by `os.constants.dlopen`.


'are' is too definitive, they are only exported when available.

bnoordhuis · 2017-05-22T17:23:51Z

src/node.cc

+    // libuv does not support passing flags to dlopen,
+    // so we nee to open code the uv_dlopen implementation.
+    dlerror();
+    lib.errmsg = NULL;


bnoordhuis · 2017-05-22T17:24:23Z

src/node.cc

+      const char* errmsg = dlerror();
+
+      if (errmsg) {
+        lib.errmsg = strdup(errmsg);


This relies a bit too much on libuv implementation details to my taste.

bnoordhuis · 2017-05-22T17:26:02Z

test/parallel/test-binding-constants.js

@@ -9,7 +9,7 @@ assert.deepStrictEqual(
 );

 assert.deepStrictEqual(
-  Object.keys(constants.os).sort(), ['UV_UDP_REUSEADDR', 'errno', 'signals']
+  Object.keys(constants.os).sort(), ['UV_UDP_REUSEADDR', 'dlopen', 'errno', 'signals']


Long line. Please run make test or make lint, that's what they are for.

ezequielgarcia · 2017-06-14T18:56:52Z

@bnoordhuis How about this: https://github.com/ezequielgarcia/node-libao/blob/dlopen-hack/index.js#L7 ?

Not sure how problematic, or how nice, it is call process.dlopen directly in the module's index.js, but it does solve the problem and only needs an extra arg in dlopen. No require changes.

bnoordhuis · 2017-06-15T11:14:00Z

@ezequielgarcia Something like that, although I don't understand why you pass in module. I would have expected a two-arg overload of process.dlopen().

ezequielgarcia · 2017-06-15T14:17:43Z

@bnoordhuis You say an overload that uses the current module? since process is a native module, I thought it wasn't possible to overload a function from it. Maybe I was wrong.

refack · 2017-09-07T13:51:13Z

@jasnell node-review has flagged you as still "Requesting Changes". AFAICT your comments have been addressed. Can I consider your objections dismissed and land this?

jasnell · 2017-09-07T14:10:21Z

I haven't yet done a thorough re-review, so I can't sign off but you can consider my initial review addressed and clear it.

mhdawson · 2017-09-07T15:58:36Z

@gireeshpunathil thanks for offering to work with @ezequielgarcia to get the test working for AIX. Can you open a new issue to track that effort ?

and +1 to going ahead and landing

thefourtheye

Have a few questions, that's all.

thefourtheye · 2017-09-07T21:26:10Z

doc/api/os.md

+### dlopen Constants
+
+If available on the operating system, the following constants
+are exported in `os.constants.dlopen`. See dlopen(3) for detailed


Care to link http://man7.org/linux/man-pages/man3/dlopen.3.html?

Isn't it done automagically? I think we already discussed this.

My bad. Yes, tools/doc/html.js will take care of this.

thefourtheye · 2017-09-07T21:28:27Z

doc/api/process.md

+
+* `module` {Object}
+* `filename` {string}
+* `flags` {integer}.


Nit: Dot at the end is not required.

Also, it would be better to specify the default value of this optional parameter, which is kDefaultFlags. Right now crypto module's .md has examples for this.

Hm, the default value is platform specific.

All the more important to document it I believe.

thefourtheye · 2017-09-07T21:29:32Z

doc/api/process.md

+The `process.dlopen()` method allows to dynamically load shared
+objects. It is primarily used by `require()` to load
+C++ Addons, and should not be used directly, except in special
+cases. In other words, [`require()`][] should be prefered over


thefourtheye · 2017-09-07T21:36:49Z

src/node.cc

@@ -2498,36 +2502,96 @@ struct node_module* get_linked_module(const char* name) {
  return mp;
 }

-// DLOpen is process.dlopen(module, filename).
+struct DLib {


Should this be a struct? If we keep it as a class and accept the values of filename_ and errmsg_ in the constructor, then we don't have to expose all the members as public right?

Already discussed. See above.

I couldn't find the actual discussion corresponding to this. I know it is exhausting to search through it, but if you could find it, please point me to it. I'll try again when I get back home.

On #12794 (comment) it was suggested to do a class. So I did it and commented
#12794 (comment). Later, #12794 (comment), where it was suggested to do a struct instead.

Sorry, I didn't check properly then.

thefourtheye · 2017-09-07T21:37:58Z

src/node.cc

+    uv_dlclose(&lib_);
+  }
+#endif
+};


Isn't calling Close necessary in the destructor? I see that we are explicitly calling Close everywhere, but I thought cleaning up in destructor is the best practice.

The handle should only be closed on error. It needs to stay open on success, otherwise the shared object would be unloaded again.

We are not closing everywhere. The handle is only closed on error. See #12794 (comment)

thefourtheye · 2017-09-07T21:42:48Z

test/addons/dlopen-ping-pong/test.js

+
+if (common.isWindows) {
+  common.skip('dlopen global symbol loading is not supported on this os.');
+  return;


The returns are not necessary. skip will take care of it.

thefourtheye · 2017-09-07T21:44:14Z

test/addons/dlopen-ping-pong/test.js

+
+const bindingPath = require.resolve(`./build/${common.buildType}/binding`);
+process.dlopen(module, bindingPath,
+               os.constants.dlopen.RTLD_NOW | os.constants.dlopen.RTLD_GLOBAL);


These variables are defined in an if macro guard. If they are not available in the current OS, then this test should be skipped, right?

I guess so.

thefourtheye · 2017-09-07T21:45:14Z

test/parallel/test-binding-constants.js

@@ -9,7 +9,8 @@ assert.deepStrictEqual(
 );

 assert.deepStrictEqual(
-  Object.keys(constants.os).sort(), ['UV_UDP_REUSEADDR', 'errno', 'signals']
+  Object.keys(constants.os).sort(), ['UV_UDP_REUSEADDR', 'dlopen', 'errno',
+                                     'signals']


Style Nit: The second array would look better in the second line I guess.

Let's add constants for dlopen flags, which are needed for dlopen's flag passing, implemented in next commit. Signed-off-by: Ezequiel Garcia <[email protected]>

bnoordhuis · 2017-09-08T11:24:19Z

src/node.cc

+    uv_dlclose(&lib_);
+  }
+#endif
+};


The handle should only be closed on error. It needs to stay open on success, otherwise the shared object would be unloaded again.

bnoordhuis · 2017-09-08T11:24:58Z

src/node.cc

+
+  const char* GetError() {
+    return errmsg_.c_str();
+  }


Since these methods are simple wrappers and only used once, I'd simply inline them at the call site.

You mean like this Local<String> errmsg = OneByteString(env->isolate(), dlib.errmsg_.c_str()); @bnoordhuis ?

bnoordhuis · 2017-09-08T11:26:26Z

src/node.cc

+  void Close() {
+    uv_dlclose(&lib_);
+  }
+#endif


Can you change this to #endif // !__POSIX__? (Note the two spaces before the //.)

This commit introduces 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). Signed-off-by: Ezequiel Garcia <[email protected]>

ezequielgarcia · 2017-09-08T11:44:18Z

Just pushed another branch addressing @bnoordhuis and @thefourtheye comments.
Changes:

s/prefered/preferred
documented dlopen default flags.
removed return after common.skip in tests.
removed GetError and Handle wrappers from DLib struct, and instead access the members directly.
added comment in #endif.

refack · 2017-09-08T12:23:31Z

CI: https://ci.nodejs.org/job/node-test-pull-request/10000/
(ohh boy 5 figure job ID)

refack · 2017-09-08T21:15:16Z

Landed in 5f22375
(after only 4 months and two prior PR attempts)
🍾

* 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 · 2017-09-10T19:48:40Z

@refack thanks for taking care of this!

* 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]>

* **async_hooks** * Older experimental `async_hooks` APIs have been removed [[`d731369b1d`](d731369b1d)] **(SEMVER-MAJOR)** [#14414](#14414) * **Errors** * Multiple built in modules have been migrated to use static error codes * **Domains** * The long deprecated `.dispose()` method has been removed [[`602fd36d95`](602fd36d95)] **(SEMVER-MAJOR)** [#15412](#15412) * **File system** * `fs.ReadStream` and `fs.WriteStream` now make use of `.destroy()` [[`e5c290bed9`](e5c290bed9)] **(SEMVER-MAJOR)** [#15407](#15407) * `fs` callbacks are now invoked with an undefined `this` context [[`2249234fee`](2249234fee)] **(SEMVER-MAJOR)** [#14645](#14645) * **HTTP** * Socket timeout is set when the socket connects [[`10be20a0e8`](10be20a0e8)] **(SEMVER-MAJOR)** [#8895](#8895) * A bug causing the request `error` event to fire twice has been fixed [[`620ba41694`](620ba41694)] **(SEMVER-MAJOR)** [#14659](#14659) * The `pipe` method on `OutgoingMessage` has been disabled [[`156549d8ff`](156549d8ff)] **(SEMVER-MAJOR)** [#14358](#14358) * **HTTP/2** * The `--expose-http2` command-line argument is no longer required [[`f55ee6e24a`](f55ee6e24a)] **(SEMVER-MAJOR)** [#15535](#15535) * **Internationalization** * The `Intl.v8BreakIterator` class has been removed [[`668ad44922`](668ad44922)] **(SEMVER-MAJOR)** [#15238](#15238) * **OS** * `os.EOL` is now read-only [[`f6caeb9526`](f6caeb9526)] **(SEMVER-MAJOR)** [#14622](#14622) * **Process** * It is now possible to pass additional flags to `dlopen` [[`5f22375922`](5f22375922)] **(SEMVER-MAJOR)** [#12794](#12794) * **Timers** * Using a timeout duration larger than 32-bits will now emit a warning [[`ce3586da31`](ce3586da31)] **(SEMVER-MAJOR)** [#15627](#15627) * **TLS** * `parseCertString` has been deprecated [[`468110b327`](468110b327)] **(SEMVER-MAJOR)** [#14249](#14249) * Type-checking for `key`, `cert`, and `ca` options has been added [[`a7dccd040d`](a7dccd040d)] **(SEMVER-MAJOR)** [#14807](#14807)

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. module Issues and PRs related to the module subsystem. labels May 2, 2017

mscdex reviewed May 2, 2017

View reviewed changes

addaleax reviewed May 2, 2017

View reviewed changes

jasnell requested changes May 2, 2017

View reviewed changes

ezequielgarcia mentioned this pull request May 5, 2017

New dlopen flags API #12848

Closed

ezequielgarcia force-pushed the dlopen_with_flags branch from 1991d15 to d343194 Compare May 11, 2017 23:54

jasnell reviewed May 12, 2017

View reviewed changes

jasnell previously requested changes May 12, 2017

View reviewed changes

gireeshpunathil previously requested changes May 18, 2017

View reviewed changes

ezequielgarcia force-pushed the dlopen_with_flags branch from d343194 to 23aa881 Compare May 22, 2017 16:08

mscdex reviewed May 22, 2017

View reviewed changes

bnoordhuis requested changes May 22, 2017

View reviewed changes

gireeshpunathil mentioned this pull request Sep 7, 2017

Fix / enable test/addons/dlopen-ping-pong/test.js for AIX #15243

Closed

thefourtheye reviewed Sep 7, 2017

View reviewed changes

constants: add dlopen flags

6ec4386

Let's add constants for dlopen flags, which are needed for dlopen's flag passing, implemented in next commit. Signed-off-by: Ezequiel Garcia <[email protected]>

ezequielgarcia force-pushed the dlopen_with_flags branch from 69e911e to baf1be2 Compare September 8, 2017 11:26

bnoordhuis approved these changes Sep 8, 2017

View reviewed changes

ezequielgarcia force-pushed the dlopen_with_flags branch from baf1be2 to 3ca00e7 Compare September 8, 2017 11:41

thefourtheye approved these changes Sep 8, 2017

View reviewed changes

refack closed this Sep 8, 2017

refack added the notable-change PRs with changes that should be highlighted in changelogs. label Sep 8, 2017

refack mentioned this pull request Sep 8, 2017

Tracking Issue: Node.js 9.0.0 #14391

Closed

refack added this to the 9.0.0 milestone Sep 8, 2017

gireeshpunathil mentioned this pull request Sep 11, 2017

aix: enable runtime linking #15286

Closed

3 tasks

targos mentioned this pull request Oct 18, 2017

test: clean up string concatenation #15820

Closed

4 tasks

jasnell mentioned this pull request Oct 30, 2017

Proposal: 2017-10-31, Version 9.0.0 (Current) #15135

Merged

ezequielgarcia mentioned this pull request Feb 1, 2018

Switch the backend from mpg123 to libao TooTallNate/node-speaker#111

Open


		<!-- type=misc -->

		require() supports an optional parameter than can be used to pass

Add process.dlopenFlags #12794

Add process.dlopenFlags #12794

Conversation

ezequielgarcia commented May 2, 2017 • edited Loading

Checklist

Affected core subsystem(s)

mscdex May 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezequielgarcia May 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mscdex May 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasnell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezequielgarcia commented May 2, 2017 • edited Loading

sam-github commented May 4, 2017

ezequielgarcia commented May 4, 2017

gireeshpunathil commented May 7, 2017

ezequielgarcia commented May 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasnell May 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasnell left a comment

Choose a reason for hiding this comment

gireeshpunathil left a comment

Choose a reason for hiding this comment

ezequielgarcia commented May 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mscdex May 22, 2017 • edited Loading

Choose a reason for hiding this comment

bnoordhuis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezequielgarcia commented Jun 14, 2017

bnoordhuis commented Jun 15, 2017

ezequielgarcia commented Jun 15, 2017

refack commented Sep 7, 2017

jasnell commented Sep 7, 2017

mhdawson commented Sep 7, 2017

thefourtheye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezequielgarcia Sep 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezequielgarcia commented May 2, 2017 •

edited

Loading

mscdex May 2, 2017 •

edited

Loading

ezequielgarcia May 22, 2017 •

edited

Loading

mscdex May 2, 2017 •

edited

Loading

ezequielgarcia commented May 2, 2017 •

edited

Loading

jasnell May 12, 2017 •

edited

Loading

ezequielgarcia commented May 22, 2017 •

edited

Loading

mscdex May 22, 2017 •

edited

Loading

ezequielgarcia Sep 7, 2017 •

edited

Loading