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)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta1

People

(Reporter: karlt, Assigned: karlt)

References

Details

Attachments

(1 file)

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.
Blocks: 343416
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.
(That said, rather than removing the PR_UnloadLibrary, I'd comment it out, with a comment explaining why.)
(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.
Attached patch don't unload, and delay loading (deleted) — Splinter Review
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???).
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+
Keywords: checkin-needed
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
Depends on: 460979
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: