After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 604867 - Don't call IceCloseConnection() behind libSM's back
Don't call IceCloseConnection() behind libSM's back
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2009-12-17 23:04 UTC by Owen Taylor
Modified: 2010-01-14 21:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't call IceCloseConnection() behind libSM's back (1.68 KB, patch)
2009-12-17 23:04 UTC, Owen Taylor
none Details | Review
Don't call IceCloseConnection() behind libSM's back (1.71 KB, patch)
2010-01-03 16:14 UTC, Owen Taylor
committed Details | Review

Description Owen Taylor 2009-12-17 23:04:19 UTC
We're getting a lot of duplicates filed against F12 with a
crash where SmcSetProperties() is getting called on exit
after the ICE connection is already closed.

 https://bugzilla.redhat.com/show_bug.cgi?id=539905

I think this is happening within the GDM session with
a sequence like:

 - gnome-session killed with SIGTERM
 - metacity gets an IO error on the ICE connection,
   and calls IceConnectionClosed() on the ICE connection
 - metacity killed with SIGTERM, quits the mainloop
 - meta_finalize() calls meta_session_shutdown which
   calls SmcSetProperties(), which tries to use the closed
   ICE connection and segfaults

This patch switches to calling disconnect() which calls
SmcCloseConnection() and marks the connection as closed,
at which point meta_session_shutdown() won't try to set
properties.

Since I haven't managed to reproduce the bug (it's really hard
because everything is quiting at once), I'm not absolutely
positive I've diagnosed it correctly, but in testing, this
version seems to quit at least as cleanly as the old version.
Comment 1 Owen Taylor 2009-12-17 23:04:21 UTC
Created attachment 149946 [details] [review]
Don't call IceCloseConnection() behind libSM's back

The ICE connection is opened by libSM; we can't just close it when
we get an IOError on the ICE connection; instead call SmcCloseConnection()
and mark the connection as closed. This will prevent a segfault if we
exit out of the metacity main loop and get to meta_finalize().
Comment 2 Owen Taylor 2010-01-03 16:14:34 UTC
Created attachment 150738 [details] [review]
Don't call IceCloseConnection() behind libSM's back

The last attempt at this patch didn't actually remove the
call to IceCloseConnection(). The end result was
deterministically closing the connection twice. Here's
a better version.
Comment 3 Thomas Thurman 2010-01-14 21:53:08 UTC
Review of attachment 150738 [details] [review]:

Looks good.  Applied.