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

HttpInput reopen/recycle cleanup #7284

Closed
sbordet opened this issue Dec 14, 2021 · 3 comments · Fixed by #7291
Closed

HttpInput reopen/recycle cleanup #7284

sbordet opened this issue Dec 14, 2021 · 3 comments · Fixed by #7291
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@sbordet
Copy link
Contributor

sbordet commented Dec 14, 2021

Jetty version(s)
10+

Description
HttpInput.recycle() is an empty method.
Interceptors are only destroyed when HttpInput is reopened, so their resources may linger around for a long time.
It would be better to destroy interceptors ASAP in recycle() and reset the state in reopen().

Also, HttpInput.reopen() calls AsyncContentProducer.recycle() which is confusing.
Consider having both recycle() and reopen() in AsyncContentProducer and forward from HttpInput.

These 2 methods should be javadoc'ed to clarify when they are called and what actions they do.

@sbordet sbordet added the Bug For general bugs on Jetty side label Dec 14, 2021
sbordet referenced this issue Dec 14, 2021
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
lorban added a commit that referenced this issue Dec 15, 2021
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
lorban added a commit that referenced this issue Dec 15, 2021
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
lorban added a commit that referenced this issue Dec 15, 2021
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
lorban added a commit that referenced this issue Dec 15, 2021
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
lorban added a commit that referenced this issue Dec 15, 2021
…een recycle and reopen

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
lorban added a commit that referenced this issue Dec 15, 2021
…ntent is produced between recycle and reopen

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@winne42
Copy link

winne42 commented Dec 16, 2021

These 2 methods should be javadoc'ed to clarify when they are called and what actions they do.

It would be really great to have some documentation about HttpInput's and AsyncContentProducer's methods and how they work together! Looking forward to this.

lorban added a commit that referenced this issue Dec 16, 2021
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban lorban linked a pull request Dec 16, 2021 that will close this issue
@lorban
Copy link
Contributor

lorban commented Dec 16, 2021

@winne42 HttpInput and ContentProducer are some of the most complicated pieces of Jetty's internals. They basically implement a finite-state machine that acts as a bridge between the servlet's InputStream (which obeys another finite-state machine because of async mode) and another finite-state machine in the HttpChannel layer (which abstracts away the differences between HTTP 1 and 2 and other protocols). They are considered internal APIs subject to change at any time.

Besides the existing javadoc, there are these PlantUML diagrams that document the interactions between the three layers as well as the state transitions for the most important API calls:

For async servlets:

https://github.com/eclipse/jetty.project/blob/2231e6496e87ae95c4c6db3d5c45c493ac525749/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput_async.puml

and for blocking servlets:

https://github.com/eclipse/jetty.project/blob/2231e6496e87ae95c4c6db3d5c45c493ac525749/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput_blocking.puml

Plus a summary of the ContentProducer state machine:

https://github.com/eclipse/jetty.project/blob/2231e6496e87ae95c4c6db3d5c45c493ac525749/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInputState.puml

That's pretty much all I can say succinctly on this subject, except that I would like to insist on the fact that you should not depend on any of this, except to satisfy your curiosity.

I hope this helps.

lorban added a commit that referenced this issue Dec 16, 2021
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@winne42
Copy link

winne42 commented Dec 17, 2021

@lorban Thank you so much for the detailed explanation and the links! I can see that you consider this to be internal implementation details. As someone interested in Jetty interceptors I just need to understand when readFrom() and destroy() are being called and what's commonly done in each - just the info you added to the JavaDoc in #7296 <3.

(the standard LMGTFY approach doesn't work - when googling "jetty interceptor" I get zero helpful hits on the first result page, but instead two extensive guides [1, 2] how to use interceptors in Jersey... :-D)

lorban added a commit that referenced this issue Jan 10, 2022
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants