Last Comment Bug 351609 - PR_UnloadLibrary always calls NSUnLinkModule on lib->dlh on Mac OS X.
: PR_UnloadLibrary always calls NSUnLinkModule on lib->dlh on Mac OS X.
Status: RESOLVED FIXED
:
Product: NSPR
Classification: Components
Component: NSPR (show other bugs)
: 4.6
: PowerPC Mac OS X
: -- normal (vote)
: 4.6.4
Assigned To: Wan-Teh Chang
: Wan-Teh Chang
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-06 17:04 PDT by Wan-Teh Chang
Modified: 2006-10-06 16:37 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (967 bytes, patch)
2006-09-06 17:06 PDT, Wan-Teh Chang
mark: review+
Details | Diff | Splinter Review

Description Wan-Teh Chang 2006-09-06 17:04:11 PDT
On Mac OS X, PRLibrary has several fields:

    CFragConnectionID           connection;
    CFBundleRef                 bundle;
    CFMutableDictionaryRef      wrappers;
    const struct mach_header*   image;
    NSModule                    dlh;

because we support loading several kinds of shared libraries.
Only one of these fields is non-NULL.

In PR_UnloadLibrary, we always call NSUnLinkModule on lib->dlh:

    result = NSUnLinkModule(lib->dlh, NSUNLINKMODULE_OPTION_NONE) ? 0 : -1;

probably because lib->dlh is the first supported or "primary"
kind of shared library.

If lib->dlh is NULL, this call causes 'result' to be set to -1, and
eventually causes PR_UnloadLibrary to fail, even though the right
kind of shared library (lib->connection, lib->bundle, lib->wrappers,
or lib->image) is successfully unloaded.
Comment 1 Wan-Teh Chang 2006-09-06 17:06:51 PDT
Created attachment 237026 [details] [diff] [review]
Proposed patch

The fix is to test lib->dlh for non-NULL, just like we
test lib->connection, lib->bundle, and lib->wrappers
for non-NULL later in the PR_UnloadLibrary function.
Comment 2 Mark Mentovai 2006-09-08 13:03:11 PDT
Comment on attachment 237026 [details] [diff] [review]
Proposed patch

Looks good.  The description in comment 0 is slightly misleading, but the patch is fine.

(When you say "only one of these fields is non-NULL," it's not quite true: wrappers and connection are set together.  Also, lib->image cannot be unloaded.)
Comment 3 Wan-Teh Chang 2006-09-12 11:30:30 PDT
You're right, Mark.  Thanks for the code review.

I checked in the patch on the NSPR trunk (NSPR 4.7) and
the NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.9 alpha).

Checking in prlink.c;
/cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c,v  <--  prlink.c
new revision: 3.92; previous revision: 3.91
done

Checking in prlink.c;
/cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c,v  <--  prlink.c
new revision: 3.51.2.37; previous revision: 3.51.2.36
done
Comment 4 Wan-Teh Chang 2006-10-06 16:37:00 PDT
I checked in the patch on the NSPR_4_6_BRANCH (NSPR 4.6.4).

Checking in prlink.c;
/cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c,v  <--  prlink.c
new revision: 3.81.2.5; previous revision: 3.81.2.4
done

Note You need to log in before you can comment on or make changes to this bug.