GNOME Bugzilla – Bug 334469
Large files can be garbled by libsoup server
Last modified: 2006-05-30 18:41:08 UTC
Please describe the problem: In various improbable race conditions, which are difficult to reproduce, but which do occur ocassionally for large files together with unpause/pause, pages can be garbled. The cause is easier to describe in terms of control flow. io_read completes and directly calls io_write (case SOUP_MESSAGE_IO_STATE_DONE). io_write blocks on part of a large file's body having written the header and some of the body (in write_data) and so returns. The main loop selects the fd for read and recalls io_read. As the read state is case SOUP_MESSAGE_IO_STATE_DONE, the write is restarted in case SOUP_MESSAGE_IO_STATE_HEADERS. The write_data writes the header, then writes the start of the body again. This causes garbage in the response. Steps to reproduce: See above. You will need data which is large enough for write body to block half way through, and to pause, set the response, and then unpause. I don't have a minimal test case, but I do have a fix. It would take days to produce a minimal test case. Actual results: Garbage sent as server response. Expected results: Currect data sent as server response. Does this happen every time? Yes. Other information: Sorry this is so arcane. What you can do is in the io_read switch change the following case to case SOUP_MESSAGE_IO_STATE_DONE: if (io->mode == SOUP_MESSAGE_IO_SERVER) { if(io->write_state==SOUP_MESSAGE_IO_STATE_NOT_STARTED) io->write_state = SOUP_MESSAGE_IO_STATE_HEADERS; io_write (sock, msg); } else soup_message_io_finished (msg); return; ie, insert the if(io->write_state==SOUP_MESSAGE_IO_STATE_NOT_STARTED) around the io->write_state assignment.
I can't figure out how io_read() would get called after read_state was set to IO_STATE_DONE. In particular, SoupSocket should only be polling for readability after it returns a SOUP_SOCKET_WOULD_BLOCK, and every code path that gets one of those immediately returns from io_read, leaving read_state the same as it was before. Are you using "100 Continue" or any other 1xx responses? Are you ever pausing the transfer during the read stage rather than during the write stage? Is your server code visible somewhere public where I could look at it? (I'm not necessarily saying this patch is wrong, I'm just afraid that it might be wallpapering over a larger bug somewhere else in soup-message-io. But if we can't find anything, I'll take your patch as is.)
I may be pausing during read, I certainly do pause quite extensively (the http messages are part of a synchronisation system), and I do seem to remember unpause being on the stack. I didn't realise this was an issue: is this part of the API contract? But I'll try to produce a minimal test-case and add it here.
You certainly *should* be allowed to pause while reading, but I don't know that anyone ever has before, and when I looked through the code, it seemed like that might be one way to cause an extra io_read() that shouldn't be there.
Can you try the patch at http://bugzilla.gnome.org/attachment.cgi?id=66143&action=view (from bug 342640) and see if that fixes the problem for you?
Actually, there's a new version of the patch now: http://bugzilla.gnome.org/attachment.cgi?id=66305&action=view
This checkin broke simple pause/unpause for me 2006-04-10 Dan Winship <danw@novell.com> * libsoup/soup-message-io.c (io_write, io_read): g_return_if_fail if these get called after the IO is done. This isn't supposed to happen, but apparently does. Workaround for #334469. I get: (process:8153): libsoup-CRITICAL **: io_read: assertion `io->read_state != SOUP_MESSAGE_IO_STATE_DONE' failed Even the testcase in tests/simple-proxy.c breaks in this way (once you add a g_thread_init (NULL); call to make it work), so I don't think this is my fault.
Alex, can you try the patch mentioned above? (http://bugzilla.gnome.org/attachment.cgi?id=66305&action=view). It definitely fixes simple-proxy.
fixed in cvs and libsoup 2.2.93 (going out imminently)
That fixes my app too. Thanks.