Commit 01872a6d authored by Stefan Eissing's avatar Stefan Eissing
Browse files

On the trunk: 

mod_http2: fixed problem of forgotten requests when number of connections larger than number of workers. Some code refactor, threads now queued in slot order for vanity reasons, so that the segfaults are more likely at the top and not the bottom.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1789520 13f79535-47bb-0310-9956-ffa450edef68
parent 5a4a1733

modules/http2/h2_conn.c

+5 −1
Original line number Diff line number Diff line
@@ -125,7 +125,11 @@ apr_status_t h2_conn_child_init(apr_pool_t *pool, server_rec *s) 
        minw = max_threads_per_child;

    }

    if (maxw <= 0) {

        maxw = minw;

        /* As a default, this seems to work quite well under mpm_event. 

         * For people enabling http2 under mpm_prefork, start 4 threads unless 

         * configured otherwise. People get unhappy if their http2 requests are 

         * blocking each other. */

        maxw = H2MAX(3 * minw / 2, 4);

    }

    

    idle_secs = h2_config_geti(config, H2_CONF_MAX_WORKER_IDLE_SECS);

modules/http2/h2_mplx.c

+20 −13
Original line number Diff line number Diff line
@@ -673,6 +673,20 @@ apr_status_t h2_mplx_reprioritize(h2_mplx *m, h2_stream_pri_cmp *cmp, void *ctx) 
    return status;

}



static void register_if_needed(h2_mplx *m) 

{

    if (!m->is_registered && !h2_iq_empty(m->q)) {

        apr_status_t status = h2_workers_register(m->workers, m); 

        if (status == APR_SUCCESS) {

            m->is_registered = 1;

        }

        else {

            ap_log_cerror(APLOG_MARK, APLOG_ERR, status, m->c, APLOGNO()

                          "h2_mplx(%ld): register at workers", m->id);

        }

    }

}



apr_status_t h2_mplx_process(h2_mplx *m, struct h2_stream *stream, 

                             h2_stream_pri_cmp *cmp, void *ctx)

{
@@ -688,17 +702,13 @@ apr_status_t h2_mplx_process(h2_mplx *m, struct h2_stream *stream, 
            if (h2_stream_is_ready(stream)) {

                /* already have a response */

                check_data_for(m, stream->id);

                ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, m->c,

                ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, m->c,

                              H2_STRM_MSG(stream, "process, add to readyq")); 

            }

            else {

                h2_iq_add(m->q, stream->id, cmp, ctx);

                if (!m->is_registered) {

                    if (h2_workers_register(m->workers, m) == APR_SUCCESS) {

                        m->is_registered = 1;

                    }

                }

                ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, m->c,

                register_if_needed(m);                

                ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, m->c,

                              H2_STRM_MSG(stream, "process, added to q")); 

            }

        }
@@ -901,11 +911,7 @@ void h2_mplx_task_done(h2_mplx *m, h2_task *task, h2_task **ptask) 
            /* caller wants another task */

            *ptask = next_stream_task(m);

        }

        if (!m->is_registered && !h2_iq_empty(m->q)) {

            if (h2_workers_register(m->workers, m) == APR_SUCCESS) {

                m->is_registered = 1;

            }

        }

        register_if_needed(m);

        leave_mutex(m, acquired);

    }

}
@@ -1042,6 +1048,7 @@ apr_status_t h2_mplx_idle(h2_mplx *m) 
                status = unschedule_slow_tasks(m);

            }

        }

        register_if_needed(m);

        leave_mutex(m, acquired);

    }

    return status;

modules/http2/h2_workers.c

+66 −50
Original line number Diff line number Diff line
@@ -69,6 +69,45 @@ static void push_slot(h2_slot **phead, h2_slot *slot) 
    }

}



static void* APR_THREAD_FUNC slot_run(apr_thread_t *thread, void *wctx);



static apr_status_t activate_slot(h2_workers *workers, h2_slot *slot) 

{

    apr_status_t status;

    

    slot->workers = workers;

    slot->aborted = 0;

    slot->task = NULL;

    if (!slot->not_idle) {

        status = apr_thread_cond_create(&slot->not_idle, workers->pool);

        if (status != APR_SUCCESS) {

            push_slot(&workers->free, slot);

            return status;

        }

    }

    

    /* thread will either immediately start work or add itself

     * to the idle queue */

    apr_thread_create(&slot->thread, workers->thread_attr, slot_run, slot, 

                      workers->pool);

    if (!slot->thread) {

        push_slot(&workers->free, slot);

        return APR_ENOMEM;

    }

    

    ++workers->worker_count;

    return APR_SUCCESS;

}



static apr_status_t add_worker(h2_workers *workers)

{

    h2_slot *slot = pop_slot(&workers->free);

    if (slot) {

        return activate_slot(workers, slot);

    }

    return APR_EAGAIN;

}



static void wake_idle_worker(h2_workers *workers) 

{

    h2_slot *slot = pop_slot(&workers->idle);
@@ -77,6 +116,9 @@ static void wake_idle_worker(h2_workers *workers) 
        apr_thread_cond_signal(slot->not_idle);

        apr_thread_mutex_unlock(workers->lock);

    }

    else if (workers->dynamic) {

        add_worker(workers);

    }

}



static void cleanup_zombies(h2_workers *workers)
@@ -144,12 +186,13 @@ static apr_status_t get_next(h2_slot *slot) 
        }

        

        apr_thread_mutex_lock(workers->lock);

        ++workers->idle_workers;

        cleanup_zombies(workers);

        if (slot->next == NULL) {



        ++workers->idle_workers;

        push_slot(&workers->idle, slot);

        }

        apr_thread_cond_wait(slot->not_idle, workers->lock);

        --workers->idle_workers;



        apr_thread_mutex_unlock(workers->lock);

    }

    return APR_EOF;
@@ -190,36 +233,6 @@ static void* APR_THREAD_FUNC slot_run(apr_thread_t *thread, void *wctx) 
    return NULL;

}



static apr_status_t activate_slot(h2_workers *workers)

{

    h2_slot *slot = pop_slot(&workers->free);

    if (slot) {

        apr_status_t status;

        

        slot->workers = workers;

        slot->aborted = 0;

        slot->task = NULL;

        if (!slot->not_idle) {

            status = apr_thread_cond_create(&slot->not_idle, workers->pool);

            if (status != APR_SUCCESS) {

                push_slot(&workers->free, slot);

                return status;

            }

        }

        

        apr_thread_create(&slot->thread, workers->thread_attr, slot_run, slot, 

                          workers->pool);

        if (!slot->thread) {

            push_slot(&workers->free, slot);

            return APR_ENOMEM;

        }



        ++workers->worker_count;

        return APR_SUCCESS;

    }

    return APR_EAGAIN;

}



static apr_status_t workers_pool_cleanup(void *data)

{

    h2_workers *workers = data;
@@ -255,7 +268,7 @@ h2_workers *h2_workers_create(server_rec *s, apr_pool_t *server_pool, 
    apr_status_t status;

    h2_workers *workers;

    apr_pool_t *pool;

    int i;

    int i, n;



    ap_assert(s);

    ap_assert(server_pool);
@@ -278,7 +291,7 @@ h2_workers *h2_workers_create(server_rec *s, apr_pool_t *server_pool, 
    workers->max_workers = max_workers;

    workers->max_idle_secs = (idle_secs > 0)? idle_secs : 10;



    status = h2_fifo_create(&workers->mplxs, pool, workers->max_workers);

    status = h2_fifo_create(&workers->mplxs, pool, 2 * workers->max_workers);

    if (status != APR_SUCCESS) {

        return NULL;

    }
@@ -300,22 +313,29 @@ h2_workers *h2_workers_create(server_rec *s, apr_pool_t *server_pool, 
                                     APR_THREAD_MUTEX_DEFAULT,

                                     workers->pool);

    if (status == APR_SUCCESS) {        

        int n = workers->nslots = workers->max_workers;

        n = workers->nslots = workers->max_workers;

        workers->slots = apr_pcalloc(workers->pool, n * sizeof(h2_slot));

        if (workers->slots == NULL) {

            workers->nslots = 0;

            status = APR_ENOMEM;

        }

        for (i = 0; i < n; ++i) {

            workers->slots[i].id = i;

        }

    }

    if (status == APR_SUCCESS) {

        workers->free = &workers->slots[0];

        for (i = 0; i < workers->nslots-1; ++i) {

            workers->slots[i].next = &workers->slots[i+1];

            workers->slots[i].id = i;

        /* we activate all for now, TODO: support min_workers again.

         * do this in reverse for vanity reasons so slot 0 will most

         * likely be at head of idle queue. */

        n = workers->max_workers;

        for (i = n-1; i >= 0; --i) {

            status = activate_slot(workers, &workers->slots[i]);

        }

        while (workers->worker_count < workers->max_workers 

               && status == APR_SUCCESS) {

            status = activate_slot(workers);

        /* the rest of the slots go on the free list */

        for(i = n; i < workers->nslots; ++i) {

            push_slot(&workers->free, &workers->slots[i]);

        }

        workers->dynamic = (workers->worker_count < workers->max_workers);

    }

    if (status == APR_SUCCESS) {

        apr_pool_pre_cleanup_register(pool, workers, workers_pool_cleanup);
@@ -326,11 +346,7 @@ h2_workers *h2_workers_create(server_rec *s, apr_pool_t *server_pool, 


apr_status_t h2_workers_register(h2_workers *workers, struct h2_mplx *m)

{

    apr_status_t status;

    if ((status = h2_fifo_try_push(workers->mplxs, m)) != APR_SUCCESS) {

        ap_log_error(APLOG_MARK, APLOG_TRACE3, status, workers->s,

                     "h2_workers: unable to push mplx(%ld)", m->id);

    } 

    apr_status_t status = h2_fifo_push(workers->mplxs, m);

    wake_idle_worker(workers);

    return status;

}

modules/http2/h2_workers.h

+2 −1
Original line number Diff line number Diff line
@@ -44,7 +44,8 @@ struct h2_workers { 
    int idle_workers;

    int max_idle_secs;

    

    unsigned int aborted : 1;

    int aborted;

    int dynamic;



    apr_threadattr_t *thread_attr;

    int nslots;