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 581879 - GFileMonitor crash: emit_cb not handling monitor disposal in signal handlers
GFileMonitor crash: emit_cb not handling monitor disposal in signal handlers
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2009-05-08 14:57 UTC by Robert Bragg
Modified: 2009-05-20 12:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to ref the monitor before signaling pending changes (1.16 KB, patch)
2009-05-08 15:08 UTC, Robert Bragg
none Details | Review
patch for emit_cb to detect disposal of the monitor using a weak pointer (2.13 KB, patch)
2009-05-08 15:19 UTC, Robert Bragg
none Details | Review

Description Robert Bragg 2009-05-08 14:57:50 UTC
While emit_cb is iterating through the pending_file_changes it is possible for a signal handler to remove the last reference and dispose the monitor.  If there are more pending_file_changes emit_cb will try and emit a signal with the disposed monitor.
Comment 1 Robert Bragg 2009-05-08 15:08:38 UTC
Created attachment 134261 [details] [review]
patch to ref the monitor before signaling pending changes

This patch for gio/gfilemonitor.c:emit_cb fixes the crash I was hitting by just ref'ing the monitor before sending the signals and unref'ing before leaving emit_cb.

I saw there were specific comments in gfilemonitor.c:emit_in_idle regarding referencing of monitors:
/* We don't ref here - instead dispose will free any
 * pending idles.
 */

so I'm not sure, but this patch may break some intended semantics since a monitor may receive some signals after the user believes they have released the last reference.
Comment 2 Robert Bragg 2009-05-08 15:19:26 UTC
Created attachment 134262 [details] [review]
patch for emit_cb to detect disposal of the monitor using a weak pointer

This alternative patch uses a weak pointer to detect if a monitor has been disposed and maintains the invariable that if the user releases their last reference then no more pending changes will be signaled on that monitor.
Comment 3 Alexander Larsson 2009-05-20 12:25:09 UTC
The first patch is right, the comment is just about not keeping the monitor alive just for the case of running the idle (if its otherwise unreffed before reaching idle), if the idle is actually running we want to ref it.

Applied.