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

Fix multiple dispatch problem with async servlet timeouts. #54

Merged
merged 2 commits into from Dec 1, 2016

Conversation

spand
Copy link
Contributor

@spand spand commented Nov 30, 2016

Servlet containers with async context will redispatch after a timeout. In Jettys case the default seems to be 30 secs. I think it is part of the three steps discribed here: https://docs.oracle.com/javaee/6/api/javax/servlet/AsyncContext.html

The provided testcase fails without the fix.

This might also be the cause of what we are seeing in #52

Copy link
Contributor

@cy6erGn0m cy6erGn0m left a comment

Choose a reason for hiding this comment

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

Does it run about a second with fix?


@Test
fun foo(){
val port = 44003
Copy link
Contributor

Choose a reason for hiding this comment

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

fixed port number is not good, see findFreePort

class MultipleDispatchOnTimeout {

@Test
fun foo(){
Copy link
Contributor

Choose a reason for hiding this comment

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

test name

@spand
Copy link
Contributor Author

spand commented Dec 1, 2016

Yes. About 2.8 seconds.

However, it is mostly a proof of concept for the problem. Maybe the correct solution would be to set the timeout to 0 globally ? Or handled some other way ?

@mathiasbn
Copy link

A temporary workaround:
System.setProperty("org.eclipse.jetty.server.HttpChannelState.DEFAULT_TIMEOUT","0")

@cy6erGn0m cy6erGn0m merged commit 604d834 into ktorio:master Dec 1, 2016
@cy6erGn0m
Copy link
Contributor

@spand merged, thanks!

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.

None yet

3 participants