Skip to content
This repository has been archived by the owner on May 29, 2023. It is now read-only.

[fix]: memory leak in mod_dyups when ssl session reuse is enabled. #143

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

timebug
Copy link

@timebug timebug commented Aug 13, 2020

Hi, @yzprofile

Issue #67 open for a long time, this problem also appeared in our online environment recently. I see @RocFang fix it just skip set_session and save_session function in PR #141, but after that will make proxy ssl session reuse not work.

So, I redesigned set_session and save_session function, the new solution will solve memory leak issue and keep ssl session reuse work:

  1. Save the pointer address to ssl_session data in the upstream related variable, not pre request related variable such as dyups_ctx, avoid losing after request complete.
  2. Use the upstream related memory pool to allocate ssl_session memory, it will destroy when free dynamic upstream or exit process.
  3. Free up extra ssl session memory which allocate at OpenSSL immediately after ngx_ssl_set_session(pc->connection, ssl_session) success.

Steps to reproduce memory leak issue:

server {
    listen   8080;

    location / {
        proxy_ssl_verify        off;
        proxy_ssl_session_reuse on;

        proxy_pass https://$host;
        proxy_http_version 1.1;
        proxy_set_header Connection "";   
    }
}

server {
    listen 8088 ssl;
    ssl_certificate     ssl/test.crt;
    ssl_certificate_key ssl/test.key;

    location / {
        return 301;
    }
}

server {
    listen 8089 ssl;
    ssl_certificate     ssl/test.crt;
    ssl_certificate_key ssl/test.key;

    location / {
        return 302;
    }
}

server {
    listen 8081;
    location / {
        dyups_interface;
    }
}

case1: (keepalive or no keepalive)

  1. curl -H "Host: dyhost" -d "server 127.0.0.1:8089;server 127.0.0.1:8088;" 127.0.0.1:8081/upstream/dyhost -v
  2. for i in {1..10000}; do curl -H "host: dyhost" 127.0.0.1:8080 -v; d

case2: (keepalive or no keepalive)

  1. for i in {1..10000}; do curl -H "Host: dyhost" -d "server 127.0.0.1:8089;server 127.0.0.1:8088;" 127.0.0.1:8081/upstream/dyhost -v; done
  2. for i in {1..10000}; do curl -H "host: dyhost" 127.0.0.1:8080 -v; d

@timebug
Copy link
Author

timebug commented Aug 14, 2020

ping @chobits

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant