GNOME Bugzilla – Bug 659324
G_OUTPUT_STREAM_SPLICE_CLOSE_TARGET doesn't mark the output stream as closed
Last modified: 2011-09-19 18:34:13 UTC
Under certain conditions, it's possible for a GLocalFileOutputStream to call close() twice on the same FD, which is bad and causes pain and suffering for other streams. Example set of conditions: • Two threads: main thread and GIO worker thread. The main thread calls g_output_stream_splice_async() on an arbitrary GInputStream and a GLocalFileOutputStream which wraps FD number f. The main thread passes G_OUTPUT_STREAM_SPLICE_CLOSE_TARGET to the splice function. The worker thread does all the splicing stuff, then comes to close the target stream (the GLocalFileOutputStream). The code in g_output_stream_real_splice() calls the output stream's close_fn() directly for this. This calls close(f). *This doesn't set the ->closed flag on the output stream.* Shortly afterwards, the main thread unrefs the GAsyncResult for the splice operation, which drops the last reference to the GLocalFileOutputStream. In its dispose function, it closes its FD. This calls close(f) again. If the FD f hasn't been re-used by an intermediate call to open(), everything's fine. However, if it has, we've just closed an arbitrary FD and pain results. This is what was happening in bug #658531. (Note that it's probably quite simple to reproduce this bug using a single thread and a much less complicated set of conditions; but these are the ones I was investigating, so I know they definitely reproduce the problem.) Patch coming up to fix this.
Created attachment 196809 [details] [review] Mark the output stream as closed when finishing a splice operation This should fix it. It basically makes g_output_stream_real_splice() call g_output_stream_close() on the target stream instead of the stream's close_fn() directly.
Review of attachment 196809 [details] [review]: ::: gio/goutputstream.c @@ +516,3 @@ + + stream->priv->closing = FALSE; + stream->priv->closed = TRUE; This doesn't actually return res
Pushed this (with return fix) to master and glib-2-30.
(In reply to comment #2) > Review of attachment 196809 [details] [review]: > > ::: gio/goutputstream.c > @@ +516,3 @@ > + > + stream->priv->closing = FALSE; > + stream->priv->closed = TRUE; > > This doesn't actually return res Erk. It's a bit worrying that the compiler didn't explode when it saw this. The code definitely compiled for me. Thanks for fixing & committing it.