Closed
Bug 397607
Opened 17 years ago
Closed 17 years ago
libXss.so.1 is unloaded while an XESetCloseDisplay callback is still registered
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9beta1
People
(Reporter: karlt, Assigned: karlt)
References
Details
Attachments
(1 file)
(deleted),
patch
|
roc
:
review+
roc
:
superreview+
roc
:
approval1.9+
|
Details | Diff | Splinter Review |
nsIdleServiceGTK loads and unloads libXss, but, if XScreenSaverQueryExtension
is called, then the library registers a shutdown function callback through XESetCloseDisplay.
The library exports no shutdown functions, or means to unregister this callback and so should not be unloaded until after the display(s) is closed.
Bug 394466 comments 15 to 17 have some details.
Options here are:
1. Just don't unload the library
This has an advantage that external libraries that may use libXss don't
get left in the cold.
2. Unregister the callback and call the shutdown function ourselves.
There are no appropriate interfaces provided by the library, so we would
have to hunt for the extension, using knowledge of it name, in the
ext_procs field of struct _XDisplay in Xlibint.h.
This may be assuming more about the implementation than we should.
3. Link against libXss.so rather than dlopening.
This would add an unnecessary dependency.
4. Unload the library after closing the display.
This is a bit hard to imagine, unless NSPR is modified do this when it is
shutdown (similar to the WIN16 implementation of _PR_ShutdownLinker),
possibly only when another flag is set for PR_LoadLibraryWithFlags.
Comment 1•17 years ago
|
||
I'd say just do (1), since it's easiest, although it doesn't have the advantage you mention since if other libraries use it they'd prevent it from being unloaded.
Comment 2•17 years ago
|
||
(That said, rather than removing the PR_UnloadLibrary, I'd comment it out, with a comment explaining why.)
Assignee | ||
Comment 3•17 years ago
|
||
(In reply to comment #1)
> I'd say just do (1), since it's easiest, although it doesn't have the advantage
> you mention since if other libraries use it they'd prevent it from being
> unloaded.
You are right of course, thanks: despite the name, PR_UnloadLibrary closes a handle rather than unloading the library.
Assignee | ||
Comment 4•17 years ago
|
||
NS_ASSERTION(!xsslib, "created two instances of the idle service") wasn't
quite right when xsslib is never reset, so this includes a little modification
to handle more than one (sequential or concurrently existing) instance (which
probably doesn't happen).
With that change it then made sense to only load the library when necessary,
one advantage of which is that we don't hunt for the library when first
opening the bookmarks menu (and also we don't open the library when there is
no display but widget/gtk2 is still used???).
Assignee | ||
Comment 5•17 years ago
|
||
Comment on attachment 282464 [details] [diff] [review]
don't unload, and delay loading
I'll ask roc for review as its in widget/gtk2, but dbaron feel free to comment if you wish.
Attachment #282464 -
Flags: superreview?(roc)
Attachment #282464 -
Flags: review?(roc)
Attachment #282464 -
Flags: superreview?(roc)
Attachment #282464 -
Flags: superreview+
Attachment #282464 -
Flags: review?(roc)
Attachment #282464 -
Flags: review+
Attachment #282464 -
Flags: approval1.9+
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Comment 6•17 years ago
|
||
Checking in widget/src/gtk2/nsIdleServiceGTK.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsIdleServiceGTK.cpp,v <-- nsIdleServiceGTK.cpp
new revision: 1.3; previous revision: 1.2
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M9
You need to log in
before you can comment on or make changes to this bug.
Description
•