GNOME Bugzilla – Bug 769727
Crashes on zero-length files
Last modified: 2016-08-23 10:52:31 UTC
libgdata crashes on the following abort, when reading zero-length files: libgdata:ERROR:/home/oholy/gnome/libgdata/gdata/gdata-download-stream.c:572:gdata_download_stream_read: assertion failed: ((reached_eof == FALSE && length_read > 0 && length_read <= (gssize) count && child_error == NULL) || (reached_eof == TRUE && length_read >= 0 && length_read <= (gssize) count && child_error == NULL) || (length_read == -1 && child_error != NULL)) It was originaly reported downstream: https://bugzilla.redhat.com/show_bug.cgi?id=1364782
Created attachment 333089 [details] [review] core: Fix crashes on zero-length files reached_eof is set too early and thus it may not be propagated properly in some cases, which may cause abortion when reading zero-length files.
Review of attachment 333089 [details] [review]: The patch looks good, but I would prefer to have a unit test for it please. I've just added some basic tests for GDataBuffer (commit f63af42f8ce177dd62ac5c49f6e0219005faaf72) which try to test zero-length files, but I can't reproduce the problem. Could you please update the patch to add an additional test case which reproduces the failure? Thanks!
Created attachment 333897 [details] [review] tests: Add one more test for GDataBuffer This test case reproduces the bug...
Review of attachment 333897 [details] [review]: Looks good. You could make it faster by using a GCond to synchronise the threads, rather than a timeout. ::: gdata/tests/buffer.c @@ +81,3 @@ + + /* Wait for a while to be sure that gdata_buffer_pop_data has been already called. */ + g_usleep (G_USEC_PER_SEC / 2); Use a GCond. @@ +93,3 @@ + GDataBuffer *buffer = NULL; /* owned */ + gboolean reached_eof = FALSE; + guint8 buf[1]; Add a `g_test_bug ("769727");` call here.
Created attachment 333901 [details] [review] tests: Add one more test for GDataBuffer Ok, I've added g_test_bug call, however I don't know how we can use GCond in this case. gdata_buffer_push_data has to be called after the while loop in gdata_buffer_pop_data is reached, but we can't do anything at this point...
Review of attachment 333901 [details] [review]: > gdata_buffer_push_data has to be called after the while loop in > gdata_buffer_pop_data is reached, but we can't do anything at this point... Ah, I see. In that case, you’re right; a GCond can’t be used. Can you please add a comment above test_buffer_thread_eof() which explains that the test needs to call push_data() from another thread only once pop_data() has reached its blocking loop. Thanks. OK to commit with that addition.
Comment on attachment 333089 [details] [review] core: Fix crashes on zero-length files commit 76e12748bd0e519caeeefd6e1ac7ce8086e63059
Created attachment 333976 [details] [review] tests: Add one more test for GDataBuffer commit 4b29a94a71317ffa9df282fef7f7be126687343a
Thanks!