no-pool-nginx icon indicating copy to clipboard operation
no-pool-nginx copied to clipboard

ngx_reset_pool lead a core dump

Open jinglong opened this issue 14 years ago • 1 comments

when i am using geo module for nginx0.8.54, the calling path of core dump is ngx_http_geo() ->ngx_reset_pool()-> ngx_create_pool(sizeof(ngx_pool_t), NULL) -> ngx_alloc(size, log) ->ngx_log_debug2(.. log ..) , here log is set to NULL in ngx_reset_pool. so ...

Actually, there is an other bug in ngx_reset_pool(), If caller A calls ngx_reset_pool to reset its pool (A->pool), and after that, it reuse A->poll , it will get a core dump because A->poll is destory in ngx_reset_pool but not set to the new one.

so, ngx_reset_pool(ngx_pool_t _pool) should be ngx_reset_pool(ngx_pool_t *_pool) , but its behave should be to reset the pool rather than to destroy it and get a new one. So to solve the problem totally, it would better to re-implement like this: void ngx_reset_pool(ngx_pool_t *pool) { ngx_pool_data_t *p, *tmp; ngx_pool_large_t *l;

for (l = pool->large; l ; l = l->next) {

    ngx_log_debug1(NGX_LOG_DEBUG_ALLOC, pool->log, 0, "free: %p", l->alloc);

    if (l->alloc) {
        ngx_free(l->alloc);                                                                                                            
    }   
}   

pool->large = NULL;

p = pool->d->next;
while (p != pool->d) {
    tmp = p;
    ngx_free(p->alloc);
    p->prev->next = p->next;
    p->next->prev = p->prev;
    p = p->next;
    ngx_free(tmp);
}   

ngx_free(pool->d->alloc);
pool->d->alloc = NULL;

} Please check it ,thanks.

jinglong avatar Jun 16 '11 07:06 jinglong

It indeed is a serious bug. We didn't test ngx_reset_pool() before. Thanks very much for pointing it out. Your implementation of this function is much better, and I adopt it : ). Thanks.

Ying.Guo

shrimp avatar Jun 30 '11 10:06 shrimp