191270 - Enhance PR_LoadLibrary to load a dynamic shared library (.dylib)

Status: RESOLVED FIXED

(NSPR :: NSPR, enhancement, P1)

Description

[Wan-Teh Chang]

In Mac OS X 10.1, the function NSAddImage was added,
which can be used to load a dynamic shared library
(.dylib).  An image loaded this way can't be unloaded
though.

Unfortunately, NSAddImage does not use DYLD_LIBRARY_PATH
to search for the specified dynamic shared library.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch

Comment 2

[Conrad Carlen (not reading bugmail)]

Looks good.

> +    /* No way to unload an image (lib->image) */

Do we ever attempt to unload modules (other than when we shut down)? Assuming
not, maybe there should be an assertion here in case somebody tries to?

Comment 3

[Wan-Teh Chang]

NSPR's assertion macro (PR_ASSERT) is intended for
detecting fatal internal errors, so an assertion
would not be appropriate here.  Also, the user still
needs to free the memory associated with the
PRLibrary structure even though the image cannot be
unloaded.

I agree it is important that we communicate to the
caller that an image loaded this way cannot be
unloaded, but I don't know how.  One idea is to
make PR_UnloadLibrary fail with
PR_OPERATION_NOT_SUPPORTED_ERROR, without freeing any
memory, if it is invoked on an image loaded with
NSAddImage, and add a new function PR_ReleaseLibrary
that merely frees the memory associated with the
PRLibrary structure without unloading the underlying
library/module/image.  (Note I use PR_"Release"Library
instead of PR_"Destroy"Library because PRLibrary
structures are reference counted.)  But this seems
like an overkill for an corner case that only exists
on Mac OS X.  Maybe we just document this issue?

Comment 4

[Robert Relyea]

NSS may unload a library in the middle if you delete a PKCS #11 module on the
fly. In this case it's probably a bundle, though, on the mac.

We should make sure the unload 'works most of the way' that is the library is
still removed from NSS's call table, and data is freed. If we call
PR_LoadLibrary again on the module it should succeed.

bob

Comment 5

[Wan-Teh Chang]

Do you think the ability for PR_LoadLibrary to load
a dynamic shared library (.dylib) is useful?  Do you
need it?

Comment 6

[Conrad Carlen (not reading bugmail)]

I think so.

Doug - wouldn't this solve the problem of loading libxpcom.bundle for the GRE?
(It doesn't work and that's why GRE is disabled for Mach-O)

Comment 7

[Doug Turner (:dougt)]

yes.  wtc - with this support, I can enable the GRE on OSX without any extra
work on my part.

Comment 8

[Wan-Teh Chang]

Doug, I'd like you to give this patch a test drive.
It is already checked into the tip of NSPR.  You
can do
    cvs -q co -A mozilla/nsprpub
to test this patch.  Thanks.

Comment 9

[Brian Ryner (not reading)]

wtc: to fix the problem that tinderbox showed with this patch, I'd suggest
passing NSADDIMAGE_OPTION_RETURN_ON_ERROR as an option to NSAddImage().  The
problem is that there's a 0-length file that we try to load as a dylib, and the
default behavior without that flag is to abort execution if there is an error
loading the dylib.

Comment 10

[Wan-Teh Chang]

Brian: thank you very much for tracking this down.  I've backed
out my checkin and will take a closer look at the error handling
in my patch.

Comment 11

[Wan-Teh Chang]

Attachment: Proposed patch v2

1. Pass the XXX_RETURN_ON_ERROR options to the Mac OS X
linker functions.

2. Pass the NSADDIMAGE_OPTION_WITH_SEARCHING option to
NSAddImage so that it uses DYLD_LIBRARY_PATH to search for
.dylib files.

I would appreciate it if someone can test this patch with plugins.  Thanks.

Comment 12

[Wan-Teh Chang]

Patch checked into the NSPR TIP (NSPR 4.3 Beta 2) and
NSPRPUB_PRE_4_2_CLIENT_BRANCH (mozilla 1.4alpha).

Doug, I'd appreciate it if you could test it.