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

feat(server): add http1_half_close(bool) option #1723

Merged
merged 1 commit into from
Nov 27, 2018

Conversation

seanmonstar
Copy link
Member

This option determines whether a read EOF should close the connection
automatically. The behavior was to always allow read EOF while waiting
to respond, so this option has a default of true.

Setting this option to false will allow Service futures to be canceled
as soon as disconnect is noticed.

Closes #1716

cc @luben

@luben
Copy link
Contributor

luben commented Nov 27, 2018

LGTM, let me try to test it on the broker example.

@luben
Copy link
Contributor

luben commented Nov 27, 2018

Works as expected. One suggestion: can we surface it through the server::Builder as the rest of the connection config options - I did the change locally in order to test it.

index a0e0b01..4bcd618 100644
--- a/src/server/mod.rs
+++ b/src/server/mod.rs
@@ -280,6 +280,14 @@ impl<I, E> Builder<I, E> {
         self
     }
 
+    /// Set whether half-closed connections are considered open.
+    ///
+    /// Default is `true`.
+    pub fn http1_half_close(mut self, val: bool) -> Self {
+        self.protocol.http1_half_close(val);
+        self
+    }
+

@seanmonstar
Copy link
Member Author

Yep, we can do that!

@@ -117,7 +122,7 @@ where I: AsyncRead + AsyncWrite,

fn should_error_on_eof(&self) -> bool {
// If we're idle, it's probably just the connection closing gracefully.
T::should_error_on_parse_eof() && !self.state.is_idle()
!self.state.is_idle()
Copy link
Contributor

@luben luben Nov 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes (with returning back the should_error_on_parse_eof()) the failing empty_parse_eof_does_not_return_error but I am not sure it's the right approach:

(T::should_error_on_parse_eof() || !self.state.allow_half_close) && !self.state.is_idle()

This option determines whether a read EOF should close the connection
automatically. The behavior was to always allow read EOF while waiting
to respond, so this option has a default of `true`.

Setting this option to `false` will allow Service futures to be canceled
as soon as disconnect is noticed.

Closes #1716
@seanmonstar seanmonstar merged commit 73345be into master Nov 27, 2018
@seanmonstar seanmonstar deleted the server-half-close branch November 27, 2018 20:31
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.

Response future not dropped on disconnect
2 participants