memory leak #3
Comments
|
как насчёт такого решения? diff --git a/ngx_http_response_body_module.c b/ngx_http_response_body_module.c
index 69cdd5d..3254127 100644
--- a/ngx_http_response_body_module.c
+++ b/ngx_http_response_body_module.c
@@ -19,6 +19,11 @@ typedef struct {
} ngx_http_response_body_loc_conf_t;
+typedef struct {
+ ngx_flag_t enable;
+} ngx_http_response_body_main_conf_t;
+
+
typedef struct {
ngx_http_response_body_loc_conf_t *blcf;
ngx_buf_t buffer;
@@ -32,6 +37,7 @@ static ngx_int_t
ngx_http_response_body_variable(ngx_http_request_t *r,
ngx_http_variable_value_t *v, uintptr_t data);
+static void *ngx_http_response_body_create_main_conf(ngx_conf_t *cf);
static void *ngx_http_response_body_create_loc_conf(ngx_conf_t *cf);
static char *ngx_http_response_body_merge_loc_conf(ngx_conf_t *cf, void *parent,
void *child);
@@ -49,6 +55,8 @@ static ngx_int_t ngx_http_response_body_filter_body(ngx_http_request_t *r,
static ngx_int_t ngx_http_response_body_init(ngx_conf_t *cf);
+static char *ngx_http_response_body_conf_command_check(ngx_conf_t *cf, void *post,
+ void *data);
static char *
ngx_conf_set_flag(ngx_conf_t *cf, ngx_command_t *cmd, void *conf);
@@ -80,6 +88,10 @@ ngx_http_next_body_filter_stub(ngx_http_request_t *r, ngx_chain_t *in)
}
+static ngx_conf_post_t ngx_http_response_body_conf_command_post =
+ { ngx_http_response_body_conf_command_check };
+
+
static ngx_command_t ngx_http_response_body_commands[] = {
{ ngx_string("capture_response_body"),
@@ -101,7 +113,7 @@ static ngx_command_t ngx_http_response_body_commands[] = {
ngx_conf_set_keyval,
NGX_HTTP_LOC_CONF_OFFSET,
offsetof(ngx_http_response_body_loc_conf_t, conditions),
- NULL },
+ &ngx_http_response_body_conf_command_post },
{ ngx_string("capture_response_body_if_1xx"),
NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
@@ -161,7 +173,7 @@ static ngx_http_module_t ngx_http_response_body_module_ctx = {
ngx_http_response_body_add_variables, /* preconfiguration */
ngx_http_response_body_init, /* postconfiguration */
- NULL, /* create main configuration */
+ ngx_http_response_body_create_main_conf, /* create main configuration */
NULL, /* init main configuration */
NULL, /* create server configuration */
@@ -217,6 +229,9 @@ ngx_conf_set_flag(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
blcf->capture_body = *fp;
+ ngx_http_response_body_main_conf_t *main = ngx_http_conf_get_module_main_conf(cf, ngx_http_response_body_module);
+ main->enable = 1;
+
return NGX_CONF_OK;
}
@@ -239,6 +254,9 @@ ngx_conf_set_msec(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
blcf->capture_body = 1;
+ ngx_http_response_body_main_conf_t *main = ngx_http_conf_get_module_main_conf(cf, ngx_http_response_body_module);
+ main->enable = 1;
+
return NGX_CONF_OK;
}
@@ -272,6 +290,9 @@ ngx_conf_set_keyval(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
blcf->capture_body = 1;
+ ngx_http_response_body_main_conf_t *main = ngx_http_conf_get_module_main_conf(cf, ngx_http_response_body_module);
+ main->enable = 1;
+
return NGX_CONF_OK;
}
@@ -348,6 +369,20 @@ ngx_http_response_body_request_var(ngx_conf_t *cf, ngx_command_t *cmd,
}
+static void *
+ngx_http_response_body_create_main_conf(ngx_conf_t *cf)
+{
+ ngx_http_response_body_main_conf_t *bmcf;
+
+ bmcf = ngx_pcalloc(cf->pool, sizeof(ngx_http_response_body_main_conf_t));
+
+ if (bmcf == NULL)
+ return NULL;
+
+ return bmcf;
+}
+
+
static void *
ngx_http_response_body_create_loc_conf(ngx_conf_t *cf)
{
@@ -360,9 +395,8 @@ ngx_http_response_body_create_loc_conf(ngx_conf_t *cf)
blcf->latency = NGX_CONF_UNSET_MSEC;
blcf->buffer_size = NGX_CONF_UNSET_SIZE;
- blcf->conditions = ngx_array_create(cf->pool, 10, sizeof(ngx_keyval_t));
- blcf->cv = ngx_array_create(cf->pool, 10,
- sizeof(ngx_http_complex_value_t));
+ blcf->conditions = NGX_CONF_UNSET_PTR;
+ blcf->cv = NGX_CONF_UNSET_PTR;
blcf->status_1xx = NGX_CONF_UNSET;
blcf->status_2xx = NGX_CONF_UNSET;
blcf->status_3xx = NGX_CONF_UNSET;
@@ -370,30 +404,27 @@ ngx_http_response_body_create_loc_conf(ngx_conf_t *cf)
blcf->status_5xx = NGX_CONF_UNSET;
blcf->capture_body = NGX_CONF_UNSET;
- if (blcf->conditions == NULL || blcf->cv == NULL)
- return NULL;
-
return blcf;
}
static ngx_int_t
-ngx_array_merge(ngx_array_t *l, ngx_array_t *r)
+ngx_array_merge(ngx_array_t **conf, ngx_array_t **prev, void *cmp)
{
- void *p;
-
- if (r->nelts == 0)
- return NGX_OK;
-
- if (r->size != l->size)
- return NGX_ERROR;
-
- p = ngx_array_push_n(l, r->nelts);
- if (p == NULL)
- return NGX_ERROR;
-
- ngx_memcpy(p, r->elts, r->size * r->nelts);
-
+ if (*conf == cmp || (*conf)->nelts == 0) {
+ *conf = *prev;
+ } else if (*prev != cmp && (*prev)->nelts) {
+ ngx_uint_t orig_len = (*conf)->nelts;
+ if (!ngx_array_push_n(*conf, (*prev)->nelts)) return NGX_ERROR;
+ char *elts = (*conf)->elts;
+ for (ngx_uint_t i = 0; i < orig_len; i++) {
+ ngx_memcpy(elts + (*conf)->size * ((*conf)->nelts - 1 - i), elts + (*conf)->size * (orig_len - 1 - i), (*conf)->size);
+ }
+ char *prev_elts = (*prev)->elts;
+ for (ngx_uint_t i = 0; i < (*prev)->nelts; i++) {
+ ngx_memcpy(elts + (*prev)->size * i, prev_elts + (*prev)->size * i, (*prev)->size);
+ }
+ }
return NGX_OK;
}
@@ -403,15 +434,13 @@ ngx_http_response_body_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
{
ngx_http_response_body_loc_conf_t *prev = parent;
ngx_http_response_body_loc_conf_t *conf = child;
- ngx_http_compile_complex_value_t ccv;
- ngx_http_complex_value_t *cv;
- ngx_uint_t j;
- ngx_keyval_t *kv;
ngx_conf_merge_msec_value(conf->latency, prev->latency, (ngx_msec_int_t) 0);
ngx_conf_merge_size_value(conf->buffer_size, prev->buffer_size,
(size_t) ngx_pagesize);
- if (ngx_array_merge(conf->conditions, prev->conditions) == NGX_ERROR)
+ if (ngx_array_merge(&conf->conditions, &prev->conditions, NGX_CONF_UNSET_PTR) == NGX_ERROR)
+ return NGX_CONF_ERROR;
+ if (ngx_array_merge(&conf->cv, &prev->cv, NGX_CONF_UNSET_PTR) == NGX_ERROR)
return NGX_CONF_ERROR;
ngx_conf_merge_value(conf->status_1xx, prev->status_1xx, 0);
ngx_conf_merge_value(conf->status_2xx, prev->status_2xx, 0);
@@ -420,29 +449,6 @@ ngx_http_response_body_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
ngx_conf_merge_value(conf->status_5xx, prev->status_5xx, 0);
ngx_conf_merge_value(conf->capture_body, prev->capture_body, 0);
- cv = ngx_array_push_n(conf->cv, conf->conditions->nelts);
- if (cv == NULL)
- return NGX_CONF_ERROR;
- ngx_memzero(cv, conf->cv->size * conf->cv->nalloc);
- kv = conf->conditions->elts;
-
- for (j = 0; j < conf->cv->nelts; j++) {
-
- ngx_memzero(&ccv, sizeof(ccv));
-
- ccv.cf = cf;
- ccv.value = &kv[j].key;
- ccv.complex_value = &cv[j];
- ccv.zero = 0;
-
- if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
-
- ngx_conf_log_error(NGX_LOG_ERR, cf, 0,
- "can't compile '%V'", &kv[j].key);
- return NGX_CONF_ERROR;
- }
- }
-
return NGX_CONF_OK;
}
@@ -450,6 +456,9 @@ ngx_http_response_body_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
static ngx_int_t
ngx_http_response_body_init(ngx_conf_t *cf)
{
+ ngx_http_response_body_main_conf_t *main = ngx_http_conf_get_module_main_conf(cf, ngx_http_response_body_module);
+ if (!main->enable) return NGX_OK;
+
ngx_http_next_header_filter = ngx_http_top_header_filter;
ngx_http_top_header_filter = ngx_http_response_body_filter_header;
@@ -562,6 +571,11 @@ ngx_http_response_body_filter_header(ngx_http_request_t *r)
&& ctx->blcf->latency <= ngx_http_response_body_request_time(r))
return ngx_http_next_header_filter(r);
+ if (ctx->blcf->conditions == NGX_CONF_UNSET_PTR || ctx->blcf->conditions == NULL) {
+ ngx_http_set_ctx(r, NULL, ngx_http_response_body_module);
+ return ngx_http_next_header_filter(r);
+ }
+
cv = ctx->blcf->cv->elts;
kv = ctx->blcf->conditions->elts;
@@ -639,3 +653,33 @@ ngx_http_response_body_filter_body(ngx_http_request_t *r, ngx_chain_t *in)
return ngx_http_next_body_filter(r, in);
}
+
+static char *
+ngx_http_response_body_conf_command_check(ngx_conf_t *cf, void *post, void *data)
+{
+ ngx_http_compile_complex_value_t ccv;
+ ngx_http_complex_value_t *cv;
+ ngx_http_response_body_loc_conf_t *conf = ngx_http_conf_get_module_loc_conf(cf, ngx_http_response_body_module);
+ ngx_keyval_t *kv = data;
+ if (conf->cv == NGX_CONF_UNSET_PTR || conf->cv == NULL) {
+ conf->cv = ngx_array_create(cf->pool, 1, sizeof(ngx_http_complex_value_t));
+ if (conf->cv == NULL) {
+ return NGX_CONF_ERROR;
+ }
+ }
+ cv = ngx_array_push(conf->cv);
+ if (cv == NULL) {
+ return NGX_CONF_ERROR;
+ }
+ ngx_memzero(&ccv, sizeof(ccv));
+ ccv.cf = cf;
+ ccv.value = &kv->key;
+ ccv.complex_value = cv;
+ ccv.zero = 0;
+ if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
+ ngx_conf_log_error(NGX_LOG_ERR, cf, 0,
+ "can't compile '%V'", &kv->key);
+ return NGX_CONF_ERROR;
+ }
+ return NGX_CONF_OK;
+} |
|
Ну, все это замечательно, но, к сожалению, я не готов на такие изменения, тк модуль работает уже давно в критичном проде. Выгод от этих изменений особо не вижу. Можно сделать компромиссный вариант:
|
если сделать только это, то всё равно будет много неиспользованной памяти в конфигурации, в которой много location |
|
Много понятие относительное. У нас сервис на nginx под 10gb съедает. Эта конфигурация - капля в море. |
|
вот так по капле и уходит вся память... |
|
Вы предлагаете большие оптимизации. Я не буду спорить, что они сэкономят какую-то память (здесь еще надо считать - я не уверен, что это даст какой-то визуальный эффект). Чтобы их принять, мне необходимо все тщательно проверять. На это нужно время, а время ресурс самый дорогой. |
для этого тесты можно написать, что существенно сэкономит время |
You are welcome. |
|
Вы можете создать PR. |
это же универсальный мержинг массивов!
|
Ну вот меня как-то смущает код в этой части. |
ну, сначала текущий массив увеличивается для длину предыдущего |
|
Ну:
|
а для чего она?
это у вас не важен, зато функцию можно использовать и там, где важен |
Ну мы же мержим массивы одинаковых объектов. Это хоть какая-то проверка.
Сдйлайте через 2 ngx_memmove |
|
Можно оставить вообще один массив структур где держать и CV и значения. |
мне не понятно такое ограничение. почему нельзя в http иметь две команды capture_response_body_if, а в одном location - три, а в другом - одну? |
size - это размер одного элемента массива, а не количество. |
хм... точно |
|
точнее ngx_memmove (сдвиг в conf) + ngx_memcpy (копирование из prev) |
сам себя не затрёт? если двигать на один массив из десяти элементов |
да, можно добавить для красоты if ((*conf)->size != (*prev)->size) return NGX_ERROR;хотя это и не выполнится при if (ngx_array_merge(&conf->conditions, &prev->conditions, NGX_CONF_UNSET_PTR) == NGX_ERROR)
return NGX_CONF_ERROR;
if (ngx_array_merge(&conf->cv, &prev->cv, NGX_CONF_UNSET_PTR) == NGX_ERROR) |
|
Ну как не выполняется? Если не выполняется, то это попытка смержить ну совсем разные массивы. С чего оно не выполняется? |
дак фнкция вызывается с массивами заведомо одинаковых объектов |
Здесь нельзя проверить это на стадии компиляции никак. |
|
и code style. |
сам себя не затрёт? если двигать массив из десяти элементов на одну позицию вправо |
|
memmove поддерживает копирование overlap. |
предлагаю
The text was updated successfully, but these errors were encountered: