Skip to content

cpool: don't reuse connection if we sent close#225

Merged
weissi merged 1 commit into
swift-server:masterfrom
weissi:jw-fix-outgoing-close
May 21, 2020
Merged

cpool: don't reuse connection if we sent close#225
weissi merged 1 commit into
swift-server:masterfrom
weissi:jw-fix-outgoing-close

Conversation

@weissi

@weissi weissi commented May 21, 2020

Copy link
Copy Markdown
Contributor

Motivation:

Previously, we'd only use the server's connection header to determine if
we should close the connection or not. That's wrong because if we set
connection: close ourselves, we must not reuse again.

Modification:

Set TaskHandler.closing = false if we send a close header.

Result:

More HTTP correctness.

Motivation:

Previously, we'd only use the server's connection header to determine if
we should close the connection or not. That's wrong because if we set
`connection: close` ourselves, we must not reuse again.

Modification:

Set `TaskHandler.closing = false` if we send a close header.

Result:

More HTTP correctness.
@weissi weissi requested a review from artemredkin May 21, 2020 11:33
@weissi

weissi commented May 21, 2020

Copy link
Copy Markdown
Contributor Author

hit #226 , retrying

@weissi

weissi commented May 21, 2020

Copy link
Copy Markdown
Contributor Author

@swift-server-bot test this please

@weissi

weissi commented May 21, 2020

Copy link
Copy Markdown
Contributor Author

swiftpm bug. Not filing because it's the 5.0 one, if we see this on 5.2 we should file

[437/448] Compiling Swift Module 'NIO' (66 sources)
<unknown>:0: error: missing submodule 'CNIOLinux'
/code/.build/checkouts/swift-nio/Sources/NIO/GetaddrinfoResolver.swift:24:8: error: no such module 'CNIOLinux'
import CNIOLinux
       ^

@weissi

weissi commented May 21, 2020

Copy link
Copy Markdown
Contributor Author

@swift-server-bot test this please

@weissi weissi merged commit 070c1e5 into swift-server:master May 21, 2020
@weissi weissi deleted the jw-fix-outgoing-close branch May 21, 2020 14:43
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