Skip to content

transports: http: set substream as disconnected after closing - #3900

Merged
pks-t merged 2 commits into
libgit2:masterfrom
pks-t:pks/http-close-substream-on-connect
Aug 29, 2016
Merged

transports: http: set substream as disconnected after closing#3900
pks-t merged 2 commits into
libgit2:masterfrom
pks-t:pks/http-close-substream-on-connect

Conversation

@pks-t

@pks-t pks-t commented Aug 17, 2016

Copy link
Copy Markdown
Member

When calling http_connect on a subtransport whose stream is already
connected, we first close the stream in case no keep-alive is in use.
When doing so, we do not reset the transport's connection state,
though. Usually, this will do no harm in case the subsequent connect
will succeed. But when the connection fails we are left with a
substransport which is tagged as connected but which has no valid
stream attached.

Fix the issue by resetting the subtransport's connected-state when
closing its stream in http_connect.

@carlosmn

Copy link
Copy Markdown
Member

This comes from investigating #3885 I'm assuming? I suspected something like this, but couldn't quite find where we might be going wrong.

@pks-t

pks-t commented Aug 17, 2016

Copy link
Copy Markdown
Member Author

Yeah, I spotted this one when debugging. I don't think this fixes #3885, though, as we actually segfault in stransport code. Considering we already set the transport stream to NULL, this should have no impact. Still, this is a bug which might surface in different situations.

pks-t added 2 commits August 17, 2016 11:41
When calling `http_connect` on a subtransport whose stream is already
connected, we first close the stream in case no keep-alive is in use.
When doing so, we do not reset the transport's connection state,
though. Usually, this will do no harm in case the subsequent connect
will succeed. But when the connection fails we are left with a
substransport which is tagged as connected but which has no valid
stream attached.

Fix the issue by resetting the subtransport's connected-state when
closing its stream in `http_connect`.
@pks-t
pks-t force-pushed the pks/http-close-substream-on-connect branch from dbf6545 to b145360 Compare August 17, 2016 09:41
@pks-t

pks-t commented Aug 17, 2016

Copy link
Copy Markdown
Member Author

Ugh. We also didn't reset the connected flag in http_close.

@pks-t
pks-t merged commit ace0d36 into libgit2:master Aug 29, 2016
@pks-t
pks-t deleted the pks/http-close-substream-on-connect branch October 10, 2016 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants