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 600864 - metacity calls illegal functions from sigterm handler
metacity calls illegal functions from sigterm handler
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2009-11-05 18:50 UTC by Ray Strode [halfline]
Modified: 2010-03-25 12:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bt (3.24 KB, text/plain)
2009-11-05 18:54 UTC, William Jon McCann
  Details
Don't call meta_finalize from SIGTERM handler (2.04 KB, patch)
2009-11-05 19:56 UTC, Ray Strode [halfline]
none Details | Review
Another take (2.15 KB, patch)
2009-11-05 22:41 UTC, Ray Strode [halfline]
committed Details | Review
metacity-race-autologin-cleanup.patch (508 bytes, patch)
2009-12-16 17:13 UTC, Stanislav Brabec
none Details | Review

Description Ray Strode [halfline] 2009-11-05 18:50:35 UTC
Just noticed metacity using 99% cpu.  I investigated it a bit with mccann and it's because metacity is calling meta_finalize from a sigterm handler.

This is a big no-no.  There is a set of maybe 10 - 15 functions that are legal to call from a sigterm handler.
Comment 1 William Jon McCann 2009-11-05 18:54:54 UTC
Created attachment 147031 [details]
bt
Comment 2 Ray Strode [halfline] 2009-11-05 19:56:59 UTC
Created attachment 147034 [details] [review]
Don't call meta_finalize from SIGTERM handler

It's not a legal function to call from a signal handler.
Instead defer until going back to the main loop.
Comment 3 Ray Strode [halfline] 2009-11-05 19:57:28 UTC
downstream report: https://bugzilla.redhat.com/show_bug.cgi?id=533239
Comment 4 Ray Strode [halfline] 2009-11-05 22:41:36 UTC
Created attachment 147047 [details] [review]
Another take

The previous patch doesn't work.  We never get a hangup on the fd when the pipe is closed.  This means we're leaking the fd across a fork() call somewhere in the code.

Rather than tracking that down, I've changed the patch slightly to not rely on a hangup but instead watch for an explicit write call.
Comment 5 Ray Strode [halfline] 2009-11-06 14:34:30 UTC
Note that metacity is also doing something bogus with its SIGCHLD handler.  You can't call g_signal_emit in that context either.
Comment 6 Ray Strode [halfline] 2009-11-14 15:29:23 UTC
Adding Michael because he alluded to these issues in his blog.
Comment 7 Michael Meeks 2009-11-16 13:44:34 UTC
Thanks Ray; patch looks nice - of course, the lazy man in me would use a g_idle_add - and get screwed over later when the signal was caught in the critical section of a mutex ;-) cf. mono's erstwhile problems in that regard.

Patch looks lovely & thanks for the CC :-)
Comment 8 Owen Taylor 2009-11-19 22:58:20 UTC
See bug 596200 for a patch fixing similar problems with SIGCHLD (patch there needs a little adaptation for Metacity, there's one explicit reference to mutter in a string in the code.)
Comment 9 Ray Strode [halfline] 2009-11-20 16:43:34 UTC
Comment on attachment 147047 [details] [review]
Another take

this is in master now.
Comment 10 Stanislav Brabec 2009-12-16 17:13:02 UTC
Created attachment 149856 [details] [review]
metacity-race-autologin-cleanup.patch

Attached patch is not gcc-4.4.1 -Werror compatible (warning about unused result).

Attached simple patch fixes this problem.