Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

bugfix: use of C global variables at config time could cause issues w…

…hen HUP reload failed in the middle.
  • Loading branch information...
commit 6114203d87c90cb2e13b543c79d717d7e1c43911 1 parent df9d079
@agentzh agentzh authored
View
65 src/ngx_http_memc_handler.c
@@ -25,21 +25,14 @@ static ngx_http_variable_t ngx_http_memc_variables[] = {
};
-static ngx_int_t ngx_http_memc_add_more_variables(ngx_conf_t *cf);
-static ngx_flag_t ngx_http_memc_enabled = 0;
-
static ngx_str_t ngx_http_memc_key = ngx_string("memc_key");
static ngx_str_t ngx_http_memc_cmd = ngx_string("memc_cmd");
static ngx_str_t ngx_http_memc_value = ngx_string("memc_value");
static ngx_str_t ngx_http_memc_flags = ngx_string("memc_flags");
static ngx_str_t ngx_http_memc_exptime = ngx_string("memc_exptime");
-static ngx_int_t ngx_http_memc_key_index;
-static ngx_int_t ngx_http_memc_cmd_index;
-static ngx_int_t ngx_http_memc_value_index;
-static ngx_int_t ngx_http_memc_flags_index;
-static ngx_int_t ngx_http_memc_exptime_index;
+static ngx_int_t ngx_http_memc_add_more_variables(ngx_conf_t *cf);
static ngx_int_t ngx_http_memc_variable_not_found(ngx_http_request_t *r,
ngx_http_variable_value_t *v, uintptr_t data);
static ngx_int_t ngx_http_memc_add_variable(ngx_conf_t *cf, ngx_str_t *name);
@@ -60,6 +53,7 @@ ngx_http_memc_handler(ngx_http_request_t *r)
ngx_http_upstream_t *u;
ngx_http_memc_ctx_t *ctx;
ngx_http_memc_loc_conf_t *mlcf;
+ ngx_http_memc_main_conf_t *mmcf;
ngx_str_t target;
ngx_url_t url;
@@ -74,14 +68,14 @@ ngx_http_memc_handler(ngx_http_request_t *r)
dd("memc handler");
- key_vv = ngx_http_get_indexed_variable(r, ngx_http_memc_key_index);
+ mmcf = ngx_http_get_module_main_conf(r, ngx_http_memc_module);
+ key_vv = ngx_http_get_indexed_variable(r, mmcf->key_index);
if (key_vv == NULL) {
return NGX_HTTP_INTERNAL_SERVER_ERROR;
}
- cmd_vv = ngx_http_get_indexed_variable(r, ngx_http_memc_cmd_index);
-
+ cmd_vv = ngx_http_get_indexed_variable(r, mmcf->cmd_index);
if (cmd_vv == NULL) {
return NGX_HTTP_INTERNAL_SERVER_ERROR;
}
@@ -299,8 +293,7 @@ ngx_http_memc_handler(ngx_http_request_t *r)
|| memc_cmd == ngx_http_memc_cmd_flush_all
|| memc_cmd == ngx_http_memc_cmd_delete)
{
- exptime_vv = ngx_http_get_indexed_variable(r,
- ngx_http_memc_exptime_index);
+ exptime_vv = ngx_http_get_indexed_variable(r, mmcf->exptime_index);
if (exptime_vv == NULL) {
return NGX_HTTP_INTERNAL_SERVER_ERROR;
@@ -318,8 +311,7 @@ ngx_http_memc_handler(ngx_http_request_t *r)
}
if (is_storage_cmd || memc_cmd == ngx_http_memc_cmd_get) {
- flags_vv = ngx_http_get_indexed_variable(r, ngx_http_memc_flags_index);
-
+ flags_vv = ngx_http_get_indexed_variable(r, mmcf->flags_index);
if (flags_vv == NULL) {
return NGX_HTTP_INTERNAL_SERVER_ERROR;
}
@@ -339,8 +331,7 @@ ngx_http_memc_handler(ngx_http_request_t *r)
|| memc_cmd == ngx_http_memc_cmd_incr
|| memc_cmd == ngx_http_memc_cmd_decr)
{
- value_vv = ngx_http_get_indexed_variable(r,
- ngx_http_memc_value_index);
+ value_vv = ngx_http_get_indexed_variable(r, mmcf->value_index);
if (value_vv == NULL) {
return NGX_HTTP_INTERNAL_SERVER_ERROR;
}
@@ -464,41 +455,41 @@ ngx_http_memc_valid_uint64_str(u_char *data, size_t len)
return 1;
}
+
ngx_int_t
ngx_http_memc_init(ngx_conf_t *cf)
{
- if (!ngx_http_memc_enabled) {
+ ngx_http_memc_main_conf_t *mmcf;
+
+ mmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_memc_module);
+
+ if (!mmcf->module_used) {
return NGX_OK;
}
- ngx_http_memc_key_index = ngx_http_memc_add_variable(cf,
- &ngx_http_memc_key);
- if (ngx_http_memc_key_index == NGX_ERROR) {
+ mmcf->key_index = ngx_http_memc_add_variable(cf, &ngx_http_memc_key);
+ if (mmcf->key_index == NGX_ERROR) {
return NGX_ERROR;
}
- ngx_http_memc_cmd_index = ngx_http_memc_add_variable(cf,
- &ngx_http_memc_cmd);
- if (ngx_http_memc_cmd_index == NGX_ERROR) {
+ mmcf->cmd_index = ngx_http_memc_add_variable(cf, &ngx_http_memc_cmd);
+ if (mmcf->cmd_index == NGX_ERROR) {
return NGX_ERROR;
}
- ngx_http_memc_flags_index = ngx_http_memc_add_variable(cf,
- &ngx_http_memc_flags);
- if (ngx_http_memc_flags_index == NGX_ERROR) {
+ mmcf->flags_index = ngx_http_memc_add_variable(cf, &ngx_http_memc_flags);
+ if (mmcf->flags_index == NGX_ERROR) {
return NGX_ERROR;
}
- ngx_http_memc_exptime_index = ngx_http_memc_add_variable(cf,
- &ngx_http_memc_exptime);
- if (ngx_http_memc_exptime_index == NGX_ERROR)
- {
+ mmcf->exptime_index = ngx_http_memc_add_variable(cf,
+ &ngx_http_memc_exptime);
+ if (mmcf->exptime_index == NGX_ERROR) {
return NGX_ERROR;
}
- ngx_http_memc_value_index = ngx_http_memc_add_variable(cf,
- &ngx_http_memc_value);
- if (ngx_http_memc_value_index == NGX_ERROR) {
+ mmcf->value_index = ngx_http_memc_add_variable(cf, &ngx_http_memc_value);
+ if (mmcf->value_index == NGX_ERROR) {
return NGX_ERROR;
}
@@ -521,12 +512,6 @@ ngx_http_memc_add_variable(ngx_conf_t *cf, ngx_str_t *name)
}
-void
-ngx_http_memc_set_module_enabled() {
- ngx_http_memc_enabled = 1;
-}
-
-
static ngx_int_t
ngx_http_memc_variable_not_found(ngx_http_request_t *r,
ngx_http_variable_value_t *v, uintptr_t data)
View
2  src/ngx_http_memc_handler.h
@@ -6,10 +6,8 @@
ngx_int_t ngx_http_memc_handler(ngx_http_request_t *r);
-
ngx_int_t ngx_http_memc_init(ngx_conf_t *cf);
-void ngx_http_memc_set_module_enabled(void);
#endif /* NGX_HTTP_MEMC_HANDLER_H */
View
23 src/ngx_http_memc_module.c
@@ -31,6 +31,7 @@ static char *ngx_http_memc_upstream_max_fails_unsupported(ngx_conf_t *cf,
ngx_command_t *cmd, void *conf);
static char *ngx_http_memc_upstream_fail_timeout_unsupported(ngx_conf_t *cf,
ngx_command_t *cmd, void *conf);
+static void *ngx_http_memc_create_main_conf(ngx_conf_t *cf);
static ngx_conf_bitmask_t ngx_http_memc_next_upstream_masks[] = {
@@ -124,7 +125,7 @@ static ngx_http_module_t ngx_http_memc_module_ctx = {
NULL, /* preconfiguration */
ngx_http_memc_init, /* postconfiguration */
- NULL, /* create main configuration */
+ ngx_http_memc_create_main_conf, /* create main configuration */
NULL, /* init main configuration */
NULL, /* create server configuration */
@@ -246,7 +247,8 @@ ngx_http_memc_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
static char *
ngx_http_memc_pass(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
{
- ngx_http_memc_loc_conf_t *mlcf = conf;
+ ngx_http_memc_loc_conf_t *mlcf = conf;
+ ngx_http_memc_main_conf_t *mmcf;
ngx_str_t *value;
ngx_url_t url;
@@ -267,7 +269,8 @@ ngx_http_memc_pass(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
clcf->auto_redirect = 1;
}
- ngx_http_memc_set_module_enabled();
+ mmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_memc_module);
+ mmcf->module_used = 1;
value = cf->args->elts;
@@ -374,3 +377,17 @@ ngx_http_memc_cmds_allowed(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
return NGX_CONF_OK;
}
+
+
+static void *
+ngx_http_memc_create_main_conf(ngx_conf_t *cf)
+{
+ ngx_http_memc_main_conf_t *mmcf;
+
+ mmcf = ngx_pcalloc(cf->pool, sizeof(ngx_http_memc_main_conf_t));
+ if (mmcf == NULL) {
+ return NULL;
+ }
+
+ return mmcf;
+}
View
10 src/ngx_http_memc_module.h
@@ -46,6 +46,16 @@ typedef struct {
typedef struct {
+ ngx_int_t key_index;
+ ngx_int_t cmd_index;
+ ngx_int_t value_index;
+ ngx_int_t flags_index;
+ ngx_int_t exptime_index;
+ ngx_int_t module_used;
+} ngx_http_memc_main_conf_t;
+
+
+typedef struct {
#if defined(nginx_version) && nginx_version >= 1001004
off_t rest;
#else
View
67 t/used.t
@@ -0,0 +1,67 @@
+# vi:filetype=
+
+use lib 'lib';
+use Test::Nginx::Socket;
+
+plan tests => repeat_each() * (4 * blocks());
+
+$ENV{TEST_NGINX_MEMCACHED_PORT} ||= 11211;
+
+#no_diff;
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: module not used
+--- config
+ location /t {
+ content_by_lua '
+ local v = ngx.var
+ ngx.say(v.memc_cmd, v.memc_exptime, v.memc_value, v.memc_key, v.memc_flags)
+ ';
+ }
+--- request
+ GET /t
+--- stap
+F(ngx_http_memc_add_variable) {
+ printf("memc add variable \"%s\"\n", user_string_n($name->data, $name->len))
+}
+--- stap_out
+--- response_body
+nilnilnilnilnil
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: module used
+--- config
+ location /t {
+ content_by_lua '
+ local v = ngx.var
+ ngx.say(v.memc_cmd, v.memc_exptime, v.memc_value, v.memc_key, v.memc_flags)
+ ';
+ }
+ location /bah {
+ set $memc_key foo;
+ memc_pass 127.0.0.1:11211;
+ }
+--- request
+ GET /t
+--- stap
+F(ngx_http_memc_add_variable) {
+ printf("memc add variable \"%s\"\n", user_string_n($name->data, $name->len))
+}
+--- stap_out
+memc add variable "memc_key"
+memc add variable "memc_cmd"
+memc add variable "memc_flags"
+memc add variable "memc_exptime"
+memc add variable "memc_value"
+
+--- response_body
+nilnilnilnilnil
+--- no_error_log
+[error]
+
View
192 valgrind.suppress
@@ -1,193 +1,33 @@
+# Valgrind suppression file for LuaJIT 2.0.
{
-<insert_a_suppression_name_here>
-Memcheck:Leak
-fun:malloc
-fun:ngx_alloc
-obj:*
+ Optimized string compare
+ Memcheck:Addr4
+ fun:lj_str_cmp
}
{
-<insert_a_suppression_name_here>
-Memcheck:Addr4
-fun:ngx_init_cycle
-fun:ngx_master_process_cycle
-fun:main
+ Optimized string compare
+ Memcheck:Addr1
+ fun:lj_str_cmp
}
{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_event_process_init
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Cond
- fun:ngx_conf_flush_files
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:(below main)
-}
-{
-<insert_a_suppression_name_here>
-Memcheck:Param
-epoll_ctl(event)
-fun:epoll_ctl
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_calloc
- fun:ngx_event_process_init
+ Optimized string compare
+ Memcheck:Addr4
+ fun:lj_str_new
}
{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:memalign
- fun:posix_memalign
- fun:ngx_memalign
-}
-{
- nginx-core-process-init
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_event_process_init
-}
-{
- nginx-core-crc32-init
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_crc32_table_init
- fun:main
-}
-{
- palloc_large_for_init_request
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_palloc_large
- fun:ngx_palloc
- fun:ngx_pcalloc
- fun:ngx_http_init_request
- fun:ngx_epoll_process_events
- fun:ngx_process_events_and_timers
+ Optimized string compare
+ Memcheck:Addr1
+ fun:lj_str_new
}
{
- palloc_large_for_create_temp_buf
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_palloc_large
- fun:ngx_palloc
- fun:ngx_create_temp_buf
- fun:ngx_http_init_request
- fun:ngx_epoll_process_events
- fun:ngx_process_events_and_timers
-}
-{
- accept_create_pool
- Memcheck:Leak
- fun:memalign
- fun:posix_memalign
- fun:ngx_memalign
- fun:ngx_create_pool
- fun:ngx_event_accept
- fun:ngx_epoll_process_events
- fun:ngx_process_events_and_timers
-}
-{
- create_pool_for_init_req
- Memcheck:Leak
- fun:memalign
- fun:posix_memalign
- fun:ngx_memalign
- fun:ngx_create_pool
- fun:ngx_http_init_request
- fun:ngx_epoll_process_events
- fun:ngx_process_events_and_timers
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:nss_parse_service_list
- fun:__nss_database_lookup
- obj:*
- obj:*
- fun:getgrnam_r@@GLIBC_2.2.5
- fun:getgrnam
- fun:ngx_core_module_init_conf
- fun:ngx_init_cycle
- fun:main
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:nss_parse_service_list
- fun:__nss_database_lookup
- obj:*
- obj:*
- fun:getpwnam_r@@GLIBC_2.2.5
- fun:getpwnam
- fun:ngx_core_module_init_conf
- fun:ngx_init_cycle
- fun:main
-}
-{
- <insert_a_suppression_name_here>
+ Optimized string compare
Memcheck:Cond
- fun:ngx_conf_flush_files
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_malloc
- fun:ngx_pcalloc
+ fun:lj_str_new
}
{
<insert_a_suppression_name_here>
Memcheck:Leak
fun:malloc
fun:ngx_alloc
- fun:ngx_malloc
- fun:ngx_palloc
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_malloc
- fun:ngx_create_temp_buf
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_malloc
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_create_pool
-}
-{
- <insert_a_suppression_name_here>
- Memcheck:Leak
- fun:malloc
- fun:ngx_alloc
- fun:ngx_palloc_large
+ fun:ngx_event_process_init
}
Please sign in to comment.
Something went wrong with that request. Please try again.