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

Already on GitHub? Sign in to your account

setting dlopen flags when loading .node modules #855

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants

Cf. issue 436 in joyent/node repository

I ran into the same problem reported by @springmeyer. This is a possible patch.

Wayne Volkmuth Add ability to set dlopen flags used to load .node modules
Cf. issue 436 in joyent/node repository
a77a339

@volkmuth - hey sorry to miss responding. Ya, this patch looks like it will fix my issue as well. Maybe you could create a very small test case for the nodejs guys to be able to test with? e.g. a small makefile to compile a fake dylib to illustrate the problem?

volkmuth commented Jul 6, 2011

@springmeyer - Not sure if the committers have interest in incorporating this, I posted a note on the mailing list and it didn't generate a significant response (http://groups.google.com/group/nodejs/browse_thread/thread/2f6c0d6ff6b3cf25/6f386e6a35e7edcf).

If I'm wrong and there is interest I can take a stab at doing any cleanup that's needed including a test, or e.g. @polotek's suggestion of a different API for setting the dlopen flags.

Owner

bnoordhuis commented Jul 6, 2011

Sounds like a reasonable solution to a real problem.

@pquerna Since you're already reviewing dlopen() patches, what do you think?

pquerna commented Jul 7, 2011

Is there any reason we should not just always set RTLD_NOW?

I'm not convinced we should expose this to the public API.. it seems like we really should just stop using RTLD_LAZY?

volkmuth commented Jul 7, 2011

In my case (wrapping openbabel) RTLD_NOW vs. RTLD_LAZY is irrelevant, both are
acceptable. I must have RTLD_GLOBAL set though, so always setting RTLD_NOW is
not sufficient in my case.

@springmeyer also used RTLD_GLOBAL in his code. He did say he needed RTLD_NOW,
I'm not sure why that was. Looking at his code the RTLD_GLOBAL seems to be
required too. Maybe he can clarify, but I think what he wants is RTLD_NOW |
RTLD_GLOBAL when node loads his extension, so even if you were to switch to
loading everything with RTLD_NOW that won't fix his situation either.

So a possibility is to use RTLD_GLOBAL for all extensions, but I don't think you
want to do that. You won't usually need RTLD_GLOBAL and using it pollutes the
global namespace.

You can avoid changing node or providing a public API if the extension writer
uses dlopen to load dependent libraries that need special flags. This can be
done from within the extension's init function (as @springmeyer did).

Here are a couple possible advantages to a public API:

  • loading a library with RTLD_GLOBAL pollutes the global namespace. Forcing the
    user of a node module to specify the RTLD_GLOBAL flag in some sort of public API
    brings that out into the open. If it's buried in the extension the typical node
    developer will be unaware of it and the (admittedly seemingly slight)
    possibility that it could create a problem.
  • It might make writing certain extensions easier. The typical user of a set
    of libraries will know how to build & link against the libraries. It's a small
    step from there to writing a node wrapper. However, the typical user might not
    know/understand how to use dlopen appropriately.

Dane, anything to add in support of a public API?


From: pquerna
reply@reply.github.com
To: volkmuth@yahoo.com
Sent: Wed, July 6, 2011 9:42:17 PM
Subject: Re: [node] setting dlopen flags when loading .node modules (#855)

Is there any reason we should not just always set RTLD_NOW?

I'm not convinced we should expose this to the public API.. it seems like we
really should just stop using RTLD_LAZY?

Reply to this email directly or view it on GitHub:
joyent#855 (comment)

Right, in my original ticket I think I should have said it was RTLD_GLOBAL that was most important, but when I first identified the issue all I knew was that both of them were used in the python binding for the library in question (mapnik).

I'll go test a bit more now to confirm this.

I should mention also in my case, recently this issue was also encountered for some java bindings to mapnik, and this prompted Mapnik upstream to start directly linking plugins loaded by libmapnik to libmapnik itself. This works around this issue for both nodejs, java, and avoids having to set custom flags from python, so we went with that: http://trac.mapnik.org/ticket/790.

Owner

bnoordhuis commented Jul 7, 2011

Is there any reason we should not just always set RTLD_NOW?

It slows down loading of large libraries a lot. That probably won't matter in most cases but if you dlopen() libxul.so, for example, the delay will be noticeable. We can give it a try and revert if it doesn't work out.

loading a library with RTLD_GLOBAL pollutes the global namespace

Yes. RTLD_GLOBAL is nearly always a bad idea. It also changes the order in which symbols are resolved and that leads to all kinds of obscure behaviour. Besides, I don't think you can emulate its behaviour on Windows.

If we're going to support it, it should be explicit with a big fat 'non-portable, prone to break' warning in the docs.

Can one of the admins verify this patch?

Owner

indutny commented Mar 18, 2014

No feedback for 3 years, closing.

@indutny indutny closed this Mar 18, 2014

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