Skip to content
This repository has been archived by the owner on Jul 6, 2018. It is now read-only.

http2: client session destroy also destroys associated socket #64

Closed
wants to merge 2 commits into from
Closed

http2: client session destroy also destroys associated socket #64

wants to merge 2 commits into from

Conversation

kjin
Copy link
Contributor

@kjin kjin commented May 3, 2017

Attempting to address #62 -- now calling destroy on either an Http2Session or its socket will result in calls to destroy on both.

@kjin kjin force-pushed the client-session branch from ea162f1 to 7dd3b67 Compare May 3, 2017 21:33
mcollina

This comment was marked as off-topic.

Sign in to view

@jasnell
Copy link
Member

jasnell commented May 4, 2017

This will need a test to ensure that destruction works correctly.

@kjin
Copy link
Contributor Author

kjin commented May 4, 2017

I've added the test -- please take a look to see that it's sufficient.

jasnell

This comment was marked as off-topic.

Sign in to view

jasnell

This comment was marked as off-topic.

Sign in to view

@jasnell jasnell mentioned this pull request May 5, 2017
Closed
jasnell pushed a commit that referenced this pull request May 5, 2017
@kjin @jasnell
http2: client session destroy also destroys associated socket
79342f3 
PR-URL: #64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request May 5, 2017
@kjin @jasnell
http2: add test for client.destroy behavior
4d6d609 
PR-URL: #64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented May 5, 2017

Landed with linting nits fixed.

@jasnell jasnell closed this May 5, 2017
@jasnell
Copy link
Member

jasnell commented May 5, 2017

@kjin ... there were several linting issues. make sure to run make lint to catch those before pushing :-)

@yosuke-furukawa yosuke-furukawa mentioned this pull request May 5, 2017
Closed
2 tasks
jasnell pushed a commit that referenced this pull request May 19, 2017
@kjin @jasnell
http2: client session destroy also destroys associated socket
4940d0d 
PR-URL: #64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request May 19, 2017
@kjin @jasnell
http2: add test for client.destroy behavior
2ea09d7 
PR-URL: #64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request May 31, 2017
@kjin @jasnell
http2: client session destroy also destroys associated socket
28fda31 
PR-URL: #64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request May 31, 2017
@kjin @jasnell
http2: add test for client.destroy behavior
f0f6481 
PR-URL: #64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit to jasnell/http2-1 that referenced this pull request Jun 22, 2017
@kjin @jasnell
http2: client session destroy also destroys associated socket
58a341e 
PR-URL: nodejs#64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit to jasnell/http2-1 that referenced this pull request Jun 22, 2017
@kjin @jasnell
http2: add test for client.destroy behavior
cbd7951 
PR-URL: nodejs#64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit to jasnell/http2-1 that referenced this pull request Jul 10, 2017
@kjin @jasnell
http2: client session destroy also destroys associated socket
e564fd0 
PR-URL: nodejs#64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit to jasnell/http2-1 that referenced this pull request Jul 10, 2017
@kjin @jasnell
http2: add test for client.destroy behavior
dcf7786 
PR-URL: nodejs#64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit to jasnell/http2-1 that referenced this pull request Jul 14, 2017
@kjin @jasnell
http2: client session destroy also destroys associated socket
c0c55cb 
PR-URL: nodejs#64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit to jasnell/http2-1 that referenced this pull request Jul 14, 2017
@kjin @jasnell
http2: add test for client.destroy behavior
786dbeb 
PR-URL: nodejs#64
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jasnell jasnell jasnell approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kjin @jasnell @mcollina