Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http: allow _httpMessage to be GC'ed #18865

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
7 participants
@lpinca
Copy link
Member

commented Feb 19, 2018

Set socket._httpMessage to null before emitting the 'connect' or 'upgrade' event.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

http

http: allow _httpMessage to be GC'ed
Set `socket._httpMessage` to `null` before emitting the `'connect'` or
`'upgrade'` event.
@lpinca

This comment has been minimized.

Copy link
Member Author

commented Feb 19, 2018

@lpinca lpinca added the semver-major label Feb 19, 2018

@lpinca

This comment has been minimized.

Copy link
Member Author

commented Feb 19, 2018

Tagged as semver-major for extra safety but imho it's a bug fix.

@lpinca lpinca added the memory label Feb 19, 2018

@mcollina
Copy link
Member

left a comment

LGTM. I’d call it a bugfix too.

@BridgeAR BridgeAR removed the semver-major label Feb 22, 2018

@BridgeAR

This comment has been minimized.

Copy link
Member

commented Feb 22, 2018

I agree about being a bug fix as well and removed the label accordingly. If someone does not agree, please add it back in again.

BridgeAR added a commit to BridgeAR/node that referenced this pull request Feb 22, 2018

http: allow _httpMessage to be GC'ed
Set `socket._httpMessage` to `null` before emitting the `'connect'` or
`'upgrade'` event.

PR-URL: nodejs#18865
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@BridgeAR

This comment has been minimized.

Copy link
Member

commented Feb 22, 2018

Landed in e5369e0 🎉

@BridgeAR BridgeAR closed this Feb 22, 2018

@lpinca

This comment has been minimized.

Copy link
Member Author

commented Feb 22, 2018

Yes, it would be nice to have this backported.

@lpinca lpinca deleted the lpinca:null/_httpMessage branch Feb 22, 2018

addaleax added a commit to addaleax/node that referenced this pull request Feb 26, 2018

http: allow _httpMessage to be GC'ed
Set `socket._httpMessage` to `null` before emitting the `'connect'` or
`'upgrade'` event.

PR-URL: nodejs#18865
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

addaleax added a commit that referenced this pull request Feb 26, 2018

http: allow _httpMessage to be GC'ed
Set `socket._httpMessage` to `null` before emitting the `'connect'` or
`'upgrade'` event.

PR-URL: #18865
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

@addaleax addaleax referenced this pull request Feb 27, 2018

Closed

v9.7.0 proposal #19040

MylesBorins added a commit that referenced this pull request Apr 13, 2018

http: allow _httpMessage to be GC'ed
Set `socket._httpMessage` to `null` before emitting the `'connect'` or
`'upgrade'` event.

PR-URL: #18865
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

MylesBorins added a commit that referenced this pull request Apr 13, 2018

http: allow _httpMessage to be GC'ed
Set `socket._httpMessage` to `null` before emitting the `'connect'` or
`'upgrade'` event.

PR-URL: #18865
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

@MylesBorins MylesBorins referenced this pull request Apr 13, 2018

Merged

v6.14.2 proposal #19996

@MylesBorins MylesBorins referenced this pull request May 2, 2018

Merged

v8.11.2 proposal #20478

MayaLekova added a commit to MayaLekova/node that referenced this pull request May 8, 2018

http: allow _httpMessage to be GC'ed
Set `socket._httpMessage` to `null` before emitting the `'connect'` or
`'upgrade'` event.

PR-URL: nodejs#18865
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.