NULL pointer in worker_thread()

[GoogleCodeExporter]

http://code.google.com/p/mongoose/source/browse/tags/2.8/mongoose.c#4525

mongoose 2.8   
arm-linux-gcc

static void
worker_thread(struct mg_context *ctx)
{
struct mg_connection conn;

DEBUG_TRACE((DEBUG_MGS_PREFIX "%s: thread %p starting",
__func__, (void *) pthread_self()));

(void) memset(&conn, 0, sizeof(conn));

while (get_socket(ctx, &conn.client) == TRUE) {
conn.birth_time = time(NULL);
conn.ctx = ctx;

if (conn.client.is_ssl &&
(conn.ssl = SSL_new(conn.ctx->ssl_ctx)) == NULL) {
cry(&conn, "%s: SSL_new: %d", __func__, ERRNO);
} else if (conn.client.is_ssl &&
SSL_set_fd(conn.ssl, conn.client.sock) != 1) {
cry(&conn, "%s: SSL_set_fd: %d", __func__, ERRNO);
} else if (conn.client.is_ssl && SSL_accept(conn.ssl) != 1) {
cry(&conn, "%s: SSL handshake error", __func__);
} else {
process_new_connection(&conn);
}

close_connection(&conn);
}

/* Signal master that we're done with connection and exiting */
pthread_mutex_lock(&conn.ctx->thr_mutex);  //<<<--------
conn.ctx->num_threads--;                   //<<<-------- !! Value "conn"
conn.ctx->num_idle--;                      //<<<-------- can be filled 
conn.pthread_cond_signal(&ctx->thr_cond);  //<<<-------- with zeros
assert(conn.ctx->num_threads >= 0);        //<<<--------
pthread_mutex_unlock(&conn.ctx->thr_mutex);//<<<--------

DEBUG_TRACE((DEBUG_MGS_PREFIX "%s: thread %p exiting",
__func__, (void *) pthread_self()));
}

Original issue reported on code.google.com by sanX...@gmail.com on 22 Feb 2010 at 8:29

[GoogleCodeExporter]

I got the same bug too!
And I think I known how this happened:
in function put_socket,start_thread creates a new thread for each new 
connection,and 
ctx->sq_head++,but in function worker_thread ,while() may handle all the 
connections,which makes makes ctx->sq_tail++ until ctx->sq_head == ctx->sq_tail 
in a 
single thread, and when another worker_thread comes , the while() is false (ctx-
>sq_head == ctx->sq_tail ),so the conn is just zeroed ,no other data, thus 
conn.ctx 
is invalid!  you can simply solve this problem by break the while(),while just 
allows the while() runs once,thus each worker_thread can get job to do.

Original comment by nils1...@gmail.com on 11 Mar 2010 at 3:43

[GoogleCodeExporter]

This is a mjor bug; it happens on windows too; and is a logic/threading bug. I 
fixed 
it. What happens is that a worker thread is created, but an idling thread 
steals the 
worker thread's connection; and the worker thread never enters the while loop; 
and 
bust with a null pointer exception when trying to clean up. The easy solution 
is to 
check to see if the loop was entered; and if so, continue as usual, otherwise, 
exit 
gracefully.
code follows:

static void
worker_thread(struct mg_context *ctx)
{
    struct  mg_connection   conn;
    int     processedConnection = 0;

    DEBUG_TRACE((DEBUG_MGS_PREFIX "%s: thread %p starting",
        __func__, (void *) pthread_self()));

    (void) memset(&conn, 0, sizeof(conn));

    while (get_socket(ctx, &conn.client) == TRUE) {
        conn.birth_time = time(NULL);
        conn.ctx = ctx;

        if (conn.client.is_ssl &&
            (conn.ssl = SSL_new(conn.ctx->ssl_ctx)) == NULL) {
            cry(&conn, "%s: SSL_new: %d", __func__, ERRNO);
        } else if (conn.client.is_ssl &&
            SSL_set_fd(conn.ssl, conn.client.sock) != 1) {
            cry(&conn, "%s: SSL_set_fd: %d", __func__, ERRNO);
        } else if (conn.client.is_ssl && SSL_accept(conn.ssl) != 1) {
            cry(&conn, "%s: SSL handshake error", __func__);
        } else {
            process_new_connection(&conn);
        }

        close_connection(&conn);

        processedConnection = 1;
    }

    if( processedConnection == 1 )
    {
        /* Signal master that we're done with connection and exiting */
        pthread_mutex_lock(&conn.ctx->thr_mutex);
        conn.ctx->num_threads--;
        conn.ctx->num_idle--;
        pthread_cond_signal(&conn.ctx->thr_cond);
        assert(conn.ctx->num_threads >= 0);
        pthread_mutex_unlock(&conn.ctx->thr_mutex);
    }
    else 
    {
        // connection didn't take; so handle
        // clean up ctx variables
        pthread_mutex_lock(&ctx->thr_mutex);
        ctx->num_threads--;
        ctx->num_idle--;
        pthread_cond_signal(&ctx->thr_cond);
        assert(ctx->num_threads >= 0);
        pthread_mutex_unlock(&ctx->thr_mutex);
    }

    DEBUG_TRACE((DEBUG_MGS_PREFIX "%s: thread %p exiting",
        __func__, (void *) pthread_self()));
}

Original comment by luzsun2...@gmail.com on 23 Mar 2010 at 11:23

[GoogleCodeExporter]

This has been fixed in the head, closing.
Thanks everybody!

Original comment by valenok on 21 Apr 2010 at 9:37

• Changed state: Fixed