tls: fix leak of WriteWrap+TLSWrap combination by indutny · Pull Request #9586 · nodejs/node

@indutny

Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.

Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.

To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.

`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.

@indutny added the tls

Issues and PRs related to the tls subsystem.

label

Nov 12, 2016

cjihrig

cjihrig

@indutny

bnoordhuis

@indutny

bnoordhuis

indutny added a commit that referenced this pull request

Nov 15, 2016
Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.

Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.

To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.

`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.

PR-URL: #9586
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

indutny added a commit to indutny/io.js that referenced this pull request

Nov 15, 2016
Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.

Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.

To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.

`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.

PR-URL: nodejs#9586
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins pushed a commit that referenced this pull request

Nov 16, 2016
Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.

Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.

To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.

`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.

PR-URL: #9586
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins pushed a commit that referenced this pull request

Nov 17, 2016
Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.

Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.

To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.

`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.

PR-URL: #9586
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins pushed a commit that referenced this pull request

Nov 19, 2016
Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.

Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.

To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.

`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.

PR-URL: #9586
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins pushed a commit that referenced this pull request

Nov 22, 2016
Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.

Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.

To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.

`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.

PR-URL: #9586
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This was referenced

Nov 22, 2016

MylesBorins pushed a commit that referenced this pull request

Dec 6, 2016
This LTS release comes with 144 commits. This includes 47 that are docs
related, 46 that are test related, 15 which are build / tools related,
and 9 commits which are updates to dependencies

Notable Changes:

* buffer:
  - coerce slice parameters consistently
    (Sakthipriyan Vairamani (thefourtheye))
    #9101
* deps:
    - *npm*:
      - upgrade npm to 3.10.9 (Kat Marchán)
       #9286
    - *V8*:
      - Various fixes to destructuring edge cases
        - cherry-pick 3c39bac from V8 upstream (Cristian Cavalli)
          #9138
        - cherry pick 7166503 from upstream v8 (Cristian Cavalli)
          #9173
* gtest:
  - the test reporter now outputs tap comments as yamlish
    (Johan Bergström) #9262
* inspector:
  - inspector now prompts user to use 127.0.0.1 rather than localhost
    (Eugene Ostroukhov) #9451
* tls:
  - fix memory leak when writing data to TLSWrap instance during
    handshake (Fedor Indutny)
    #9586

PR-URL: #9735

MylesBorins pushed a commit that referenced this pull request

Dec 6, 2016
This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.

Notable Changes:

The SEMVER-MINOR changes include:

* build:
  - export openssl symbols on Windows making it possible to build
    addons linked against the bundled version of openssl (Alex Hultman)
    #7576
* debugger:
  - make listen address configurable in the debugger server
    (Ben Noordhuis) #3316
* dgram:
  - generalized send queue to handle close fixing a potential throw
    when dgram socket is closed in the listening event handler.
    (Matteo Collina) #7066
* http:
  - Introduce the 451 status code "Unavailable For Legal Reasons"
    (Max Barinov) #4377
* tls:
  - introduce `secureContext` for `tls.connect` which is useful for
    caching client certificates, key, and CA certificates.
    (Fedor Indutny) #4246

Notable SEMVER-PATCH changes include:

* build:
  - introduce the configure --shared option for embedders (sxa555)
    #6994
* gtest:
  - the test reporter now outputs tap comments as yamlish
    (Johan Bergström) #9262
* src:
  - node no longer aborts when c-ares initialization fails
    (Ben Noordhuis) #8710
* tls:
  - fix memory leak when writing data to TLSWrap instance during
    handshake (Fedor Indutny)
    #9586

PR-URL: #9736

MylesBorins pushed a commit that referenced this pull request

Dec 6, 2016
This LTS release comes with 144 commits. This includes 47 that are docs
related, 46 that are test related, 15 which are build / tools related,
and 9 commits which are updates to dependencies

Notable Changes:

* buffer:
  - coerce slice parameters consistently
    (Sakthipriyan Vairamani (thefourtheye))
    #9101
* deps:
    - *npm*:
      - upgrade npm to 3.10.9 (Kat Marchán)
       #9286
    - *V8*:
      - Various fixes to destructuring edge cases
        - cherry-pick 3c39bac from V8 upstream (Cristian Cavalli)
          #9138
        - cherry pick 7166503 from upstream v8 (Cristian Cavalli)
          #9173
* gtest:
  - the test reporter now outputs tap comments as yamlish
    (Johan Bergström) #9262
* inspector:
  - inspector now prompts user to use 127.0.0.1 rather than localhost
    (Eugene Ostroukhov) #9451
* tls:
  - fix memory leak when writing data to TLSWrap instance during
    handshake (Fedor Indutny)
    #9586

PR-URL: #9735

MylesBorins pushed a commit that referenced this pull request

Dec 6, 2016
This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.

Notable Changes:

The SEMVER-MINOR changes include:

* build:
  - export openssl symbols on Windows making it possible to build
    addons linked against the bundled version of openssl (Alex Hultman)
    #7576
* debugger:
  - make listen address configurable in the debugger server
    (Ben Noordhuis) #3316
* dgram:
  - generalized send queue to handle close fixing a potential throw
    when dgram socket is closed in the listening event handler.
    (Matteo Collina) #7066
* http:
  - Introduce the 451 status code "Unavailable For Legal Reasons"
    (Max Barinov) #4377
* tls:
  - introduce `secureContext` for `tls.connect` which is useful for
    caching client certificates, key, and CA certificates.
    (Fedor Indutny) #4246

Notable SEMVER-PATCH changes include:

* build:
  - introduce the configure --shared option for embedders (sxa555)
    #6994
* gtest:
  - the test reporter now outputs tap comments as yamlish
    (Johan Bergström) #9262
* src:
  - node no longer aborts when c-ares initialization fails
    (Ben Noordhuis) #8710
* tls:
  - fix memory leak when writing data to TLSWrap instance during
    handshake (Fedor Indutny)
    #9586

PR-URL: #9736

MylesBorins pushed a commit that referenced this pull request

Dec 6, 2016
This LTS release comes with 144 commits. This includes 47 that are docs
related, 46 that are test related, 15 which are build / tools related,
and 9 commits which are updates to dependencies

Notable Changes:

* buffer:
  - coerce slice parameters consistently
    (Sakthipriyan Vairamani (thefourtheye))
    #9101
* deps:
    - *npm*:
      - upgrade npm to 3.10.9 (Kat Marchán)
       #9286
    - *V8*:
      - Various fixes to destructuring edge cases
        - cherry-pick 3c39bac from V8 upstream (Cristian Cavalli)
          #9138
        - cherry pick 7166503 from upstream v8 (Cristian Cavalli)
          #9173
* gtest:
  - the test reporter now outputs tap comments as yamlish
    (Johan Bergström) #9262
* inspector:
  - inspector now prompts user to use 127.0.0.1 rather than localhost
    (Eugene Ostroukhov) #9451
* tls:
  - fix memory leak when writing data to TLSWrap instance during
    handshake (Fedor Indutny)
    #9586

PR-URL: #9735

MylesBorins pushed a commit that referenced this pull request

Dec 6, 2016
This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.

Notable Changes:

The SEMVER-MINOR changes include:

* build:
  - export openssl symbols on Windows making it possible to build
    addons linked against the bundled version of openssl (Alex Hultman)
    #7576
* debugger:
  - make listen address configurable in the debugger server
    (Ben Noordhuis) #3316
* dgram:
  - generalized send queue to handle close fixing a potential throw
    when dgram socket is closed in the listening event handler.
    (Matteo Collina) #7066
* http:
  - Introduce the 451 status code "Unavailable For Legal Reasons"
    (Max Barinov) #4377
* tls:
  - introduce `secureContext` for `tls.connect` which is useful for
    caching client certificates, key, and CA certificates.
    (Fedor Indutny) #4246

Notable SEMVER-PATCH changes include:

* build:
  - introduce the configure --shared option for embedders (sxa555)
    #6994
* gtest:
  - the test reporter now outputs tap comments as yamlish
    (Johan Bergström) #9262
* src:
  - node no longer aborts when c-ares initialization fails
    (Ben Noordhuis) #8710
* tls:
  - fix memory leak when writing data to TLSWrap instance during
    handshake (Fedor Indutny)
    #9586

PR-URL: #9736

imyller added a commit to imyller/meta-nodejs that referenced this pull request

Dec 7, 2016
    This LTS release comes with 108 commits. This includes 30 which are doc
    related, 28 which are test related, 16 which are build / tool related,
    and 4 commits which are updates to dependencies.

    Notable Changes:

    The SEMVER-MINOR changes include:

    * build:
      - export openssl symbols on Windows making it possible to build
        addons linked against the bundled version of openssl (Alex Hultman)
        nodejs/node#7576
    * debugger:
      - make listen address configurable in the debugger server
        (Ben Noordhuis) nodejs/node#3316
    * dgram:
      - generalized send queue to handle close fixing a potential throw
        when dgram socket is closed in the listening event handler.
        (Matteo Collina) nodejs/node#7066
    * http:
      - Introduce the 451 status code "Unavailable For Legal Reasons"
        (Max Barinov) nodejs/node#4377
    * tls:
      - introduce `secureContext` for `tls.connect` which is useful for
        caching client certificates, key, and CA certificates.
        (Fedor Indutny) nodejs/node#4246

    Notable SEMVER-PATCH changes include:

    * build:
      - introduce the configure --shared option for embedders (sxa555)
        nodejs/node#6994
    * gtest:
      - the test reporter now outputs tap comments as yamlish
        (Johan Bergstrom) nodejs/node#9262
    * src:
      - node no longer aborts when c-ares initialization fails
        (Ben Noordhuis) nodejs/node#8710
    * tls:
      - fix memory leak when writing data to TLSWrap instance during
        handshake (Fedor Indutny)
        nodejs/node#9586

    PR-URL: nodejs/node#9736

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>

imyller added a commit to imyller/meta-nodejs that referenced this pull request

Dec 7, 2016
    This LTS release comes with 144 commits. This includes 47 that are docs
    related, 46 that are test related, 15 which are build / tools related,
    and 9 commits which are updates to dependencies

    Notable Changes:

    * buffer:
      - coerce slice parameters consistently
        (Sakthipriyan Vairamani (thefourtheye))
        nodejs/node#9101
    * deps:
        - *npm*:
          - upgrade npm to 3.10.9 (Kat Marchan)
           nodejs/node#9286
        - *V8*:
          - Various fixes to destructuring edge cases
            - cherry-pick 3c39bac from V8 upstream (Cristian Cavalli)
              nodejs/node#9138
            - cherry pick 7166503 from upstream v8 (Cristian Cavalli)
              nodejs/node#9173
    * gtest:
      - the test reporter now outputs tap comments as yamlish
        (Johan Bergstrom) nodejs/node#9262
    * inspector:
      - inspector now prompts user to use 127.0.0.1 rather than localhost
        (Eugene Ostroukhov) nodejs/node#9451
    * tls:
      - fix memory leak when writing data to TLSWrap instance during
        handshake (Fedor Indutny)
        nodejs/node#9586

    PR-URL: nodejs/node#9735

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>

imyller added a commit to imyller/meta-nodejs that referenced this pull request

Dec 7, 2016
    This LTS release comes with 108 commits. This includes 30 which are doc
    related, 28 which are test related, 16 which are build / tool related,
    and 4 commits which are updates to dependencies.

    Notable Changes:

    The SEMVER-MINOR changes include:

    * build:
      - export openssl symbols on Windows making it possible to build
        addons linked against the bundled version of openssl (Alex Hultman)
        nodejs/node#7576
    * debugger:
      - make listen address configurable in the debugger server
        (Ben Noordhuis) nodejs/node#3316
    * dgram:
      - generalized send queue to handle close fixing a potential throw
        when dgram socket is closed in the listening event handler.
        (Matteo Collina) nodejs/node#7066
    * http:
      - Introduce the 451 status code "Unavailable For Legal Reasons"
        (Max Barinov) nodejs/node#4377
    * tls:
      - introduce `secureContext` for `tls.connect` which is useful for
        caching client certificates, key, and CA certificates.
        (Fedor Indutny) nodejs/node#4246

    Notable SEMVER-PATCH changes include:

    * build:
      - introduce the configure --shared option for embedders (sxa555)
        nodejs/node#6994
    * gtest:
      - the test reporter now outputs tap comments as yamlish
        (Johan Bergstrom) nodejs/node#9262
    * src:
      - node no longer aborts when c-ares initialization fails
        (Ben Noordhuis) nodejs/node#8710
    * tls:
      - fix memory leak when writing data to TLSWrap instance during
        handshake (Fedor Indutny)
        nodejs/node#9586

    PR-URL: nodejs/node#9736

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>

imyller added a commit to imyller/meta-nodejs that referenced this pull request

Dec 7, 2016
    This LTS release comes with 144 commits. This includes 47 that are docs
    related, 46 that are test related, 15 which are build / tools related,
    and 9 commits which are updates to dependencies

    Notable Changes:

    * buffer:
      - coerce slice parameters consistently
        (Sakthipriyan Vairamani (thefourtheye))
        nodejs/node#9101
    * deps:
        - *npm*:
          - upgrade npm to 3.10.9 (Kat Marchan)
           nodejs/node#9286
        - *V8*:
          - Various fixes to destructuring edge cases
            - cherry-pick 3c39bac from V8 upstream (Cristian Cavalli)
              nodejs/node#9138
            - cherry pick 7166503 from upstream v8 (Cristian Cavalli)
              nodejs/node#9173
    * gtest:
      - the test reporter now outputs tap comments as yamlish
        (Johan Bergstrom) nodejs/node#9262
    * inspector:
      - inspector now prompts user to use 127.0.0.1 rather than localhost
        (Eugene Ostroukhov) nodejs/node#9451
    * tls:
      - fix memory leak when writing data to TLSWrap instance during
        handshake (Fedor Indutny)
        nodejs/node#9586

    PR-URL: nodejs/node#9735

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>