Skip to content

Commit

Permalink
[FIXED] Segfault when changing config to support dynamic module loading
Browse files Browse the repository at this point in the history
This is related to PR #17 where there is a proposal to change
the configuration to support dynamic module loading. This feature
was introduced in NGINX 1.9.11.

When compiling NGINX with config changes from PR #17 and running
with NGINX 1.14.0, the server would segfault on startup with
an issue in NATS plugin.

The issue is due to the reference to the global variable ngx_modules
which one should not do when running with dynamic modules.
The proposed changes is to use ngx_count_modules() and replace
references to ngx_modules with cycle->modules as described
[here](https://www.nginx.com/blog/nginx-dynamic-modules-how-they-work/#ngx_count_modules)

I have added a NGINX version check so that the plugin can still
be statically compiled with versions earlier than 1.9.11.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
  • Loading branch information
kozlovic committed Jul 27, 2018
1 parent 7b076f9 commit e7ce5de
Showing 1 changed file with 40 additions and 17 deletions.
57 changes: 40 additions & 17 deletions src/ngx_nats.c
Expand Up @@ -18,6 +18,8 @@
* Copyright (C) Nginx, Inc.
*/

#include <nginx.h>

#include "ngx_nats.h"
#include "ngx_nats_comm.h"

Expand Down Expand Up @@ -62,6 +64,12 @@
/* We'll go without knowing our local IP */
#endif

#if nginx_version >= 1009011
#define NATS_DYNAMIC_MODULE_SUPPORT (1)
#define NATS_NGX_MODULES(cf) ((cf)->cycle->modules)
#else
#define NATS_NGX_MODULES(cf) ngx_modules
#endif

/*---------------------------------------------------------------------------
* Forward declarations of functions for main (root) module.
Expand Down Expand Up @@ -209,6 +217,26 @@ ngx_module_t ngx_nats_core_module = {
*
*=========================================================================*/

static ngx_uint_t ngx_nats_get_modules_count(ngx_conf_t *cf)
{
#if defined(NATS_DYNAMIC_MODULE_SUPPORT)
return ngx_count_modules(cf->cycle, NGX_NATS_MODULE);
#else
ngx_uint_t i;
ngx_uint_t count = 0;

for (i = 0; ngx_modules[i]; i++) {
if (ngx_modules[i]->type != NGX_NATS_MODULE) {
continue;
}

ngx_modules[i]->ctx_index = count++;
}

return count;
#endif
}

static char *
ngx_nats_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
{
Expand All @@ -225,14 +253,9 @@ ngx_nats_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
* Count the number of the nats modules and set up their indices
*/

ngx_nats_max_module = 0;
for (i = 0; ngx_modules[i]; i++) {
if (ngx_modules[i]->type != NGX_NATS_MODULE) {
continue;
}

ngx_modules[i]->ctx_index = ngx_nats_max_module++;
}
ngx_nats_max_module = ngx_nats_get_modules_count(cf);
if (ngx_nats_max_module == 0)
return NGX_CONF_ERROR;

/*
* Setup array of configurations for NATS modules.
Expand Down Expand Up @@ -262,16 +285,16 @@ ngx_nats_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
* inside of the nats{...} block.
*/

for (i = 0; ngx_modules[i]; i++) {
if (ngx_modules[i]->type != NGX_NATS_MODULE) {
for (i = 0; NATS_NGX_MODULES(cf)[i]; i++) {
if (NATS_NGX_MODULES(cf)[i]->type != NGX_NATS_MODULE) {
continue;
}

m = ngx_modules[i]->ctx;
m = NATS_NGX_MODULES(cf)[i]->ctx;

if (m->create_conf) {
(*ctx)[ngx_modules[i]->ctx_index] = m->create_conf(cf->cycle);
if ((*ctx)[ngx_modules[i]->ctx_index] == NULL) {
(*ctx)[NATS_NGX_MODULES(cf)[i]->ctx_index] = m->create_conf(cf->cycle);
if ((*ctx)[NATS_NGX_MODULES(cf)[i]->ctx_index] == NULL) {
return NGX_CONF_ERROR;
}
}
Expand Down Expand Up @@ -299,15 +322,15 @@ ngx_nats_block(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
return NGX_CONF_ERROR;
}

for (i = 0; ngx_modules[i]; i++) {
if (ngx_modules[i]->type != NGX_NATS_MODULE) {
for (i = 0; NATS_NGX_MODULES(cf)[i]; i++) {
if (NATS_NGX_MODULES(cf)[i]->type != NGX_NATS_MODULE) {
continue;
}

m = ngx_modules[i]->ctx;
m = NATS_NGX_MODULES(cf)[i]->ctx;

if (m->init_conf) {
rv = m->init_conf(cf, (*ctx)[ngx_modules[i]->ctx_index]);
rv = m->init_conf(cf, (*ctx)[NATS_NGX_MODULES(cf)[i]->ctx_index]);
if (rv != NGX_CONF_OK) {
return rv;
}
Expand Down

0 comments on commit e7ce5de

Please sign in to comment.