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

HttpServerResponse.close() should be idempotent #139

Closed
NiteshKant opened this issue Jun 13, 2014 · 0 comments
Closed

HttpServerResponse.close() should be idempotent #139

NiteshKant opened this issue Jun 13, 2014 · 0 comments
Milestone

Comments

@NiteshKant
Copy link
Member

HttpServerResponse.close() is not idempotent and hence can not be called multiple times.

The problem with this is that since HttpConnectionHandler finally calls close at the end of request processing, a request handler can not close the response. Instead what the request handlers end up doing is call flush() on the response in a code similar to this:

new HttpServerBuilder<ByteBuf, ByteBuf>(7008, new RequestHandler<ByteBuf, ByteBuf>() {
            @Override
            public Observable<Void> handle(HttpServerRequest<ByteBuf> request, HttpServerResponse<ByteBuf> response) {
                return response.writeStringAndFlush("Hello World");
            }
        }, true)

The above code will invoke two flushes per request and hence cause two underlying socket writes as opposed to ideally a single write.

If we make HttpServerResponse.close() idempotent the above code can be changed to

new HttpServerBuilder<ByteBuf, ByteBuf>(7008, new RequestHandler<ByteBuf, ByteBuf>() {
            @Override
            public Observable<Void> handle(HttpServerRequest<ByteBuf> request, HttpServerResponse<ByteBuf> response) {
                response.writeString("Hello World");
                return response.close();
            }
        }, true)

Now, HttpConnectionHandler calling close will short-circuit (since the response is already closed) and hence will reduce one flush.

@NiteshKant NiteshKant added this to the 0.3.6 milestone Jun 13, 2014
@NiteshKant NiteshKant self-assigned this Jun 13, 2014
NiteshKant pushed a commit to NiteshKant/RxNetty that referenced this issue Jun 13, 2014
Making HttpServerResponse.close() idempotent and reducing two flushes to one for most RequestHandlers.
NiteshKant added a commit that referenced this issue Jun 13, 2014
tbak pushed a commit to tbak/RxNetty that referenced this issue Jun 13, 2014
Making HttpServerResponse.close() idempotent and reducing two flushes to one for most RequestHandlers.
@NiteshKant NiteshKant removed their assignment Aug 19, 2014
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

No branches or pull requests

1 participant