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 672775 - crash when hitting PrtSc multiple times
crash when hitting PrtSc multiple times
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: extensions
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-03-24 22:40 UTC by Matthias Clasen
Modified: 2012-03-26 15:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
stacktrace (9.37 KB, text/plain)
2012-03-24 22:40 UTC, Matthias Clasen
  Details
shell-screenshot: Remove ShellScreenshot and fall back to functions (14.07 KB, patch)
2012-03-25 06:58 UTC, Jasper St. Pierre (not reading bugmail)
rejected Details | Review
screenshot: Remove harmful empty finalizer (1.06 KB, patch)
2012-03-25 15:09 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
screenshot: Ensure that ShellScreenshot stays alive until the callback (2.57 KB, patch)
2012-03-25 15:09 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Matthias Clasen 2012-03-24 22:40:58 UTC
Created attachment 210538 [details]
stacktrace

First filed here: https://bugzilla.redhat.com//show_bug.cgi?id=806034

When hitting PrtSc or Ctrl-PrtSc multiple times in a row, the shell tends to crash. See attached stacktrace
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-03-25 06:58:33 UTC
Created attachment 210548 [details] [review]
shell-screenshot: Remove ShellScreenshot and fall back to functions

For reasons unknown, the ShellScreenshot object is being finalized
improperly, making GJS crash when it tries to call the callback.
Just remove the object for now, and figure out the real issue later.



It seems that when trying to send the Screenshot back to JS, it's already
destroyed, so gjs crashes when trying to turn the stal pointer back into
a GObject. The easy hack fix is to remove the ShellScreenshot object, as
it's just a silly wrapper around a ShellGlobal, and doesn't need to exist.

Of course, this is just a hack fix -- we should investigate why the object
is being improperly destroyed.
Comment 2 Jasper St. Pierre (not reading bugmail) 2012-03-25 15:09:09 UTC
Created attachment 210577 [details] [review]
screenshot: Remove harmful empty finalizer

We really should be chaining up in the finalizer, but instead of leaving
an empty finalize, remove it entirely.



If you don't like that fix, there's this one instead. Take your pick.
Comment 3 Jasper St. Pierre (not reading bugmail) 2012-03-25 15:09:14 UTC
Created attachment 210578 [details] [review]
screenshot: Ensure that ShellScreenshot stays alive until the callback

We've been dangling on the edge of unsafety, unnoticed, for a little while
about the reference count safety of ShellScreenshot. GJS owns the entire
reference count, so as soon as it goes out of scope it could die, causing
GJS to try and fetch the corresponding wrapper object for a stale pointer.
We haven't seen any crashes because of luck -- SpiderMonkey tries to group
together deallocations to limit GC pauses, and there isn't really a lot
of GC pressure in the duration that a screenshot happens, so we tend to
be mostly stable. But in the case that you create a lot of objects while
a screenshot is going on, by hammering the "Print Screen" button, for
example, you can destroy the GObject before the callback finishes.
Comment 4 drago01 2012-03-26 14:58:11 UTC
Review of attachment 210548 [details] [review]:

No.
Comment 5 drago01 2012-03-26 14:59:03 UTC
Review of attachment 210577 [details] [review]:

Looks good.
Comment 6 drago01 2012-03-26 14:59:19 UTC
Review of attachment 210578 [details] [review]:

Looks good.
Comment 7 Jasper St. Pierre (not reading bugmail) 2012-03-26 15:55:45 UTC
Attachment 210577 [details] pushed as 07e10fa - screenshot: Remove harmful empty finalizer
Attachment 210578 [details] pushed as 2b87bb0 - screenshot: Ensure that ShellScreenshot stays alive until the callback