[ADDED] Support for NGINX dynamic module #17

sgoldsm · 2018-07-19T22:04:59Z

Updating the module config to add the new configuration syntax for dynamic modules. The refactor was based primarily on the converting guide provided by NGINX.

While this successfully generates the new dynamic module - we're seeing a segmentation fault while reading the NATS server configuration. Still digging into the cause - but including this WIP PR with the hope that someone might be able to help us troubleshoot.

Segmentation Fault

root@abdc4db265d8:/# gdb nginx
...
(gdb) set args -c /etc/nginx/conf/nginx.conf
(gdb) run
Starting program: /usr/sbin/nginx -c /etc/nginx/conf/nginx.conf
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x00005555555867e0 in ngx_palloc ()

(gdb) backtrace
#0  0x00005555555867e0 in ngx_palloc ()
#1  0x0000555555586b1d in ngx_array_create ()
#2  0x00007ffff5242a8c in ngx_nats_core_server (cf=0x7fffffffe220, cmd=<optimized out>, conf=0x555555923a88)
    at /tmp/nginx-nats-master/src/ngx_nats.c:511
#3  0x0000555555599411 in ngx_conf_parse ()
#4  0x00007ffff5242255 in ngx_nats_block (cf=0x7fffffffe220, cmd=<optimized out>, conf=<optimized out>)
    at /tmp/nginx-nats-master/src/ngx_nats.c:288
#5  0x0000555555599411 in ngx_conf_parse ()
#6  0x0000555555596b0c in ngx_init_cycle ()
#7  0x0000555555583f0b in main ()

NGINX configuration

load_module /usr/local/nginx/modules/ngx_nats_module.so;

nats {
  server localhost:4222;

  reconnect 2s;
  ping      30s;
}

Following the guideline provided by NGINX - https://www.nginx.com/resources/wiki/extending/converting

kozlovic · 2018-07-23T17:21:21Z

@sgoldsm I am using your branch and I cannot reproduce the issue.

Here is what I did:
. Downloaded nginx from here
. Compile modules like this:

./configure --add-dynamic-module=/Users/ivan/dev/go/src/github.com/nats-io/nginx-nats/src --with-cc-opt=-I/usr/local/Cellar/openssl/1.0.2n/include/ "--with-ld-opt=-L/usr/local/Cellar/openssl/1.0.2n/lib -lssl -lcrypto" --without-http_rewrite_module
make -f objs/Makefile modules
make
make install

. Add the load_module directive and nats config before the events in ngnix.conf file:

(...)
#error_log  logs/error.log  info;

#pid        logs/nginx.pid;

load_module /usr/local/nginx/modules/ngx_nats_module.so;

nats {
  server localhost:4222;

  reconnect 2s;
  ping      30s;
}

events {
    worker_connections  1024;
}
(...)

. Started nginx: sudo /usr/local/nginx/sbin/nginx -c /usr/local/nginx/conf/nginx.conf
. Check it is started:

IvanMBP:nginx-1.14.0 ivan$ ps -ef | grep nginx
    0 99073     1   0 11:16AM ??         0:00.00 nginx: master process /usr/local/nginx/sbin/nginx -c /usr/local/nginx/conf/nginx.conf  
   -2 99074 99073   0 11:16AM ??         0:00.01 nginx: worker process  
  501 99082 90880   0 11:17AM ttys007    0:00.00 grep nginx

and

IvanMBP:nginx-1.14.0 ivan$ curl localhost:80
<!DOCTYPE html>
<html>
<head>
<title>Welcome to nginx!</title>
<style>
    body {
        width: 35em;
        margin: 0 auto;
        font-family: Tahoma, Verdana, Arial, sans-serif;
    }
</style>
</head>
<body>
<h1>Welcome to nginx!</h1>
<p>If you see this page, the nginx web server is successfully installed and
working. Further configuration is required.</p>

<p>For online documentation and support please refer to
<a href="http://nginx.org/">nginx.org</a>.<br/>
Commercial support is available at
<a href="http://nginx.com/">nginx.com</a>.</p>

<p><em>Thank you for using nginx.</em></p>
</body>
</html>
IvanMBP:nginx-1.14.0 ivan$

Any pointer to what I did "wrong" in that I can't reproduce the issue?

sgoldsm · 2018-07-24T01:12:25Z

Thanks @kozlovic for the quick feedback. If you include this Dockerfile in the root project folder - it should help reproduce the issue we're seeing.

Dockerfile

FROM ubuntu:16.04

RUN apt-get update \
 && apt-get install -y -q --no-install-recommends \
    build-essential \
    git \
    gcc \
    libc6-dev \
    make \
    libpcre3-dev \
    zlib1g-dev \
    curl \
    vim \
    apt-transport-https \
    ca-certificates \
    software-properties-common \
    libssl-dev

WORKDIR /tmp

ADD src /tmp/nginx-nats-src

RUN curl -fSL http://nginx.org/download/nginx-1.14.0.tar.gz -o nginx-1.14.0.tar.gz

RUN tar -xzvf nginx-1.14.0.tar.gz \
 && cd /tmp/nginx-1.14.0 \
 && ./configure --add-dynamic-module=/tmp/nginx-nats-src --with-cc-opt=-I/usr/include/openssl/ "--with-ld-opt=-L/usr/lib/ssl -lssl -lcrypto" --without-http_rewrite_module \
 && make -f objs/Makefile modules \
 && make \
 && make install

Build, Configure NGINX steps

docker build -t nginx-nats:local .
docker run --entrypoint /bin/bash -it nginx-nats:local
root@cd2e8e0a165f:/tmp# vim /usr/local/nginx/conf/nginx.conf
root@cd2e8e0a165f:/tmp# /usr/local/nginx/sbin/nginx -c /usr/local/nginx/conf/nginx.conf
Segmentation fault

kozlovic · 2018-07-26T02:40:02Z

@sgoldsm Thanks for the update. I can reproduce and looking more into this.

kozlovic · 2018-07-26T23:55:53Z

@sgoldsm I may have found what is causing the issue. Working on a fix. Will keep you posted.

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>

kozlovic · 2018-07-27T01:43:55Z

@sgoldsm I have a proposed fix in PR #17. If you could test that it works fine for you, that would be great. I have added a version check to be able to compile with NGINX versions earlier than when the dynamic module support was added, which I believe was in 1.9.11.

sgoldsm · 2018-07-27T13:50:46Z

Thanks @kozlovic. Really appreciate the quick turnaround. We can confirm we're able to reference the NGINX nats directive now.

kozlovic · 2018-07-27T15:10:59Z

@sgoldsm Thanks for the confirmation. I will merge the other PR first, and will then merge yours. Thank you for your contribution!

kozlovic · 2018-07-27T21:31:12Z

Removed the WIP, are you ok for us to merge?

kozlovic

LGTM

kozlovic · 2018-07-27T21:31:53Z

@sgoldsm Is it ok to merge or do you want to add more to this PR?

sgoldsm · 2018-07-27T21:32:49Z

Good to merge @kozlovic. Thanks again for the help.

sgoldsm added 2 commits July 19, 2018 17:53

Convert to dynamic module

80b209e

Following the guideline provided by NGINX - https://www.nginx.com/resources/wiki/extending/converting

Fix spacing to improve readability

2c2c077

derekcollison requested a review from kozlovic July 23, 2018 02:46

kozlovic mentioned this pull request Jul 27, 2018

[FIXED] Segfault when changing config to support dynamic module loading #18

Merged

kozlovic changed the title ~~Support for NGINX dynamic module (WIP)~~ [ADDED] Support for NGINX dynamic module Jul 27, 2018

kozlovic approved these changes Jul 27, 2018

View reviewed changes

kozlovic merged commit e82abbb into nats-io:master Jul 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADDED] Support for NGINX dynamic module #17

[ADDED] Support for NGINX dynamic module #17

sgoldsm commented Jul 19, 2018

kozlovic commented Jul 23, 2018

sgoldsm commented Jul 24, 2018

kozlovic commented Jul 26, 2018

kozlovic commented Jul 26, 2018

kozlovic commented Jul 27, 2018

sgoldsm commented Jul 27, 2018

kozlovic commented Jul 27, 2018

kozlovic commented Jul 27, 2018

kozlovic left a comment

kozlovic commented Jul 27, 2018

sgoldsm commented Jul 27, 2018

[ADDED] Support for NGINX dynamic module #17

[ADDED] Support for NGINX dynamic module #17

Conversation

sgoldsm commented Jul 19, 2018

kozlovic commented Jul 23, 2018

sgoldsm commented Jul 24, 2018

kozlovic commented Jul 26, 2018

kozlovic commented Jul 26, 2018

kozlovic commented Jul 27, 2018

sgoldsm commented Jul 27, 2018

kozlovic commented Jul 27, 2018

kozlovic commented Jul 27, 2018

kozlovic left a comment

Choose a reason for hiding this comment

kozlovic commented Jul 27, 2018

sgoldsm commented Jul 27, 2018