#46 EXCEPTION_ACCESS_VIOLATION: multithread in read_stream

[Anonymous]

I think its because read_stream gets called after the stream is freed.

-->n=stream->io_class->read(stream,io_space(&stream->io),len);
read_stream
process_connection
process_worker_sockets
poll_worker
worker_function

I was using 2 worker threads, in windows, with firefox and doing lots of browser refreshs. In attch is the stack dump and shttpd's debug.

Also in attch a possible patch to fix. It adds "(c->rem.flags & FLAG_CLOSED)" to do_close. Since this is a race it doesn't always occur, so I need help to make sure 1) reproduce the prbl, 2) check if fix makes sense and 3) check if fix actual fixes.

Thanks in advance,
Rui

[Nobody/Anonymous]

[Sergey Lyubka]

Logged In: YES
user_id=215691
Originator: NO

Please attach small files inline.

--- shttpd-1.39/src/shttpd.c.orig 2008-05-02 11:22:26.270932600 +0100
+++ shttpd-1.39/src/shttpd.c 2008-05-02 11:25:06.411557600 +0100

@@ -947,6 +947,7 @@
*/
do_close = (c->ch.connection.v_vec.len >= vec.len &&
!my_strncasecmp(vec.ptr, c->ch.connection.v_vec.ptr, vec.len)) ||
+ (c->rem.flags & FLAG_CLOSED) || (c->rem.io_class == &io_ssl) ||
(c->major_version < 1 ||
(c->major_version >= 1 && c->minor_version < 1));

(c->rem.flags & FLAG_CLOSED) makes sense to me,
(c->rem.io_class == &io_ssl) does not. Why do you force SSL connection to close?

[Sergey Lyubka]

Logged In: YES
user_id=215691
Originator: NO

Looking at the stack dump, it blows at line 1532

static void
worker_function(void *param)
{
struct worker *worker = param;

while (worker->exit_flag == 0)
poll_worker(worker, 1000 * 10);

free_list(&worker->connections, connection_desctructor);
free(worker); <--------- here
}

And this makes sense ...
Because the boss thread still has the worker in its list, and when the worker kills itself, we've got the problem.
Quick and dirty fix would be to insert the line "LL_DEL(&worker->link);"
before "free(worker);", this way we make sure we delete the worker from the boss's list. But this should be mutex-protected, really.

[Sergey Lyubka]

Logged In: YES
user_id=215691
Originator: NO

Or. If we're mutex-free guys, the worker, when it is draining, must delete all its connections, and then send the control message to the boss. Boss must free up the worker and delete it from its list.

[Nobody/Anonymous]

Logged In: NO

It blows on line 901
/* Read from underlying channel */
n = stream->io_class->read(stream, io_space(&stream->io), len); <--- EXCEPTION_ACCESS_VIOLATION
Because read_stream is being called on a stream that was freed.

Please ignore the forcing SSL close. It was for versions < 1.41 in which SSL negotiation failed when keep-alive was used.