Skip to content

Commit 794cc3d

Browse files
author
luoyudong
committed
fix: Memory leak issue of nginx module
1 parent bbcbd0e commit 794cc3d

File tree

1 file changed

+84
-13
lines changed

1 file changed

+84
-13
lines changed

nginx/ngx_http_mapcache_module.c

Lines changed: 84 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,25 @@
66
#include <apr_strings.h>
77
#include <apr_pools.h>
88

9-
9+
extern ngx_module_t ngx_http_mapcache_module;
1010
apr_pool_t *process_pool = NULL;
1111
static char *ngx_http_mapcache(ngx_conf_t *cf, ngx_command_t *cmd,
1212
void *conf);
1313

14+
typedef struct{
15+
ngx_array_t *configs; // elements: mapcache_cfg*
16+
} ngx_http_mapcache_main_conf_t;
17+
18+
// Register the cleanup function for the APR pool to the nginx configuration pool.
19+
static void ngx_http_mapcache_apr_pool_cleanup(void *data)
20+
{
21+
apr_pool_t *p = (apr_pool_t *)data;
22+
if (p)
23+
{
24+
apr_pool_destroy(p);
25+
}
26+
}
27+
1428
static ngx_command_t ngx_http_mapcache_commands[] = {
1529

1630
{
@@ -56,12 +70,23 @@ static mapcache_context* ngx_mapcache_context_clone(mapcache_context *ctx)
5670
static void *
5771
ngx_http_mapcache_create_conf(ngx_conf_t *cf)
5872
{
59-
apr_initialize();
60-
atexit(apr_terminate);
61-
apr_pool_initialize();
62-
apr_pool_create(&process_pool,NULL);
63-
mapcache_context *ctx = apr_pcalloc(process_pool, sizeof(mapcache_ngx_context));
64-
ctx->pool = process_pool;
73+
// Allocate an independent APR pool for this location and attach it to the configuration pool cleanup of nginx.
74+
apr_pool_t *loc_apr_pool = NULL;
75+
apr_initialize(); // Ensure that APR has been initialized (safe for multiple calls)
76+
apr_pool_create(&loc_apr_pool, NULL);
77+
ngx_pool_cleanup_t *cln = ngx_pool_cleanup_add(cf->pool, 0);
78+
if (cln)
79+
{
80+
cln->handler = ngx_http_mapcache_apr_pool_cleanup;
81+
cln->data = loc_apr_pool;
82+
}
83+
mapcache_ngx_context *ngctx = ngx_pcalloc(cf->pool, sizeof(mapcache_ngx_context));
84+
if (ngctx == NULL)
85+
{
86+
return NULL;
87+
}
88+
mapcache_context *ctx = (mapcache_context *)ngctx;
89+
ctx->pool = loc_apr_pool; // Use the location-specific APR pool to host the configuration memory
6590
ctx->connection_pool = NULL;
6691
mapcache_context_init(ctx);
6792
ctx->log = ngx_mapcache_context_log;
@@ -72,6 +97,27 @@ ngx_http_mapcache_create_conf(ngx_conf_t *cf)
7297
return ctx;
7398
}
7499

100+
static void *
101+
ngx_http_mapcache_create_main_conf(ngx_conf_t *cf)
102+
{
103+
// Do not create a global APR pool here or call atexit/apr_pool_initialize
104+
ngx_http_mapcache_main_conf_t *mcf;
105+
106+
mcf = ngx_pcalloc(cf->pool, sizeof(ngx_http_mapcache_main_conf_t));
107+
if (mcf == NULL)
108+
{
109+
return NULL;
110+
}
111+
112+
mcf->configs = ngx_array_create(cf->pool, 1, sizeof(mapcache_cfg *));
113+
if (mcf->configs == NULL)
114+
{
115+
return NULL;
116+
}
117+
118+
return mcf;
119+
}
120+
75121

76122
static void ngx_http_mapcache_write_response(mapcache_context *ctx, ngx_http_request_t *r,
77123
mapcache_http_response *response)
@@ -156,7 +202,7 @@ static ngx_http_module_t ngx_http_mapcache_module_ctx = {
156202
NULL, /* preconfiguration */
157203
NULL, /* postconfiguration */
158204

159-
NULL, /* create main configuration */
205+
ngx_http_mapcache_create_main_conf, /* create main configuration */
160206
NULL, /* init main configuration */
161207

162208
NULL, /* create server configuration */
@@ -168,16 +214,37 @@ static ngx_http_module_t ngx_http_mapcache_module_ctx = {
168214

169215
static ngx_int_t ngx_mapcache_init_process(ngx_cycle_t *cycle)
170216
{
217+
/* Executed when each worker process starts */
171218
apr_initialize();
172-
atexit(apr_terminate);
173-
apr_pool_initialize();
174-
apr_pool_create(&process_pool,NULL);
219+
// Do not use apr_pool_initialize or atexit
220+
apr_pool_create(&process_pool, NULL);
221+
222+
ngx_http_mapcache_main_conf_t *mcf =
223+
(ngx_http_mapcache_main_conf_t *)ngx_http_cycle_get_module_main_conf(cycle, ngx_http_mapcache_module);
224+
225+
mapcache_cfg **confs = (mapcache_cfg **)mcf->configs->elts;
226+
ngx_uint_t i;
227+
228+
mapcache_context *ctx = apr_pcalloc(process_pool, sizeof(mapcache_ngx_context));
229+
ctx->pool = process_pool;
230+
mapcache_context_init(ctx);
231+
ctx->log = ngx_mapcache_context_log;
232+
ctx->clone = ngx_mapcache_context_clone;
233+
234+
for (i = 0; i < mcf->configs->nelts; i++)
235+
{
236+
// Perform per-child initialization for each cache, using the process_pool of this worker.
237+
mapcache_cache_child_init(ctx, confs[i], process_pool);
238+
}
175239
return NGX_OK;
176240
}
177241

178242
static void ngx_mapcache_exit_process(ngx_cycle_t *cycle)
179243
{
180-
apr_pool_destroy(process_pool);
244+
if (process_pool){
245+
apr_pool_destroy(process_pool);
246+
process_pool = NULL;
247+
}
181248
}
182249

183250
ngx_module_t ngx_http_mapcache_module = {
@@ -300,14 +367,18 @@ ngx_http_mapcache(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
300367
ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "no mapcache <service>s configured/enabled, no point in continuing.");
301368
return NGX_CONF_ERROR;
302369
}
303-
mapcache_cache_child_init(ctx,ctx->config,ctx->pool);
304370
if(GC_HAS_ERROR(ctx)) {
305371
ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,ctx->get_error_message(ctx));
306372
return NGX_CONF_ERROR;
307373
}
308374
mapcache_connection_pool_create(ctx->config, &ctx->connection_pool,ctx->pool);
309375
ctx->config->non_blocking = 1;
310376

377+
/* add the mapcache config to the list of configs to be initialized on worker startup */
378+
ngx_http_mapcache_main_conf_t *mcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_mapcache_module);
379+
mapcache_cfg **pcfg = ngx_array_push(mcf->configs);
380+
*pcfg = ctx->config;
381+
311382
ngx_http_core_loc_conf_t *clcf;
312383

313384
clcf = ngx_http_conf_get_module_loc_conf(cf, ngx_http_core_module);

0 commit comments

Comments
 (0)