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.

Sign up for GitHub

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

Jump to bottom

Review HttpSender.ContentSender (and other ICB) to remove overridden succeeded method #11932

Closed
gregw opened this issue Jun 19, 2024 · 2 comments · Fixed by #11950
Closed

Review HttpSender.ContentSender (and other ICB) to remove overridden succeeded method #11932

gregw opened this issue Jun 19, 2024 · 2 comments · Fixed by #11950
Assignees
@sbordet @lorban
Labels
Bug For general bugs on Jetty side

Comments

@gregw
Copy link
Contributor

gregw commented Jun 19, 2024

Jetty version(s)
12
Jetty Environment
all
Java version/vendor (use: java -version)
all
OS type/version
all

Description

Currently the HttpSender.ContentSender class is an ICB and it overrides the succeeded() method. This method is not serialized with other ICB action, nor is there a memory barrier.
So for example, code written in onAborted (which is serialized) might be acting of a field that is changed by another thread calling succeeded().

Generally the ICB.succeeded() and ICB.failed(Throwable) methods should be considered final and overwritten only in exceptional circumstances.
@gregw gregw added the Bug For general bugs on Jetty side label Jun 19, 2024
@gregw
Copy link
Contributor Author

gregw commented Jun 19, 2024

See #11876

@gregw
Copy link
Contributor Author

gregw commented Jun 19, 2024

It looks like HttpSender.ContentSender is just one example of many in HTTP2 and HTTP3. All of these need to be reviewed and explicit onAbort handling added.

@gregw gregw mentioned this issue Jun 19, 2024
Closed
gregw added a commit that referenced this issue Jun 19, 2024
@gregw
Fixed ConnectHandler ICB usage
961393e 
See #11932
lorban added a commit that referenced this issue Jun 24, 2024
@lorban
#11932 make succeeded and failed in ICB final + introduce onSuccess
ebdee1b 
Signed-off-by: Ludovic Orban <[email protected]>
@lorban lorban mentioned this issue Jun 24, 2024
Merged
lorban added a commit that referenced this issue Jun 24, 2024
@lorban
#11932 make succeeded and failed in ICB final + introduce onSuccess
3390539 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 24, 2024
@lorban
#11932 make succeeded and failed in ICB final + introduce onSuccess
53f02ac 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 25, 2024
@lorban
#11932 fix H3 message flusher
bd3bd8e 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 25, 2024
@lorban
#11932 refactor H3 message flusher to have ICB succeeded and failed f…
05356d8 
…inal

Signed-off-by: Ludovic Orban <[email protected]>
@joakime joakime moved this to 🏗 In progress in 🧊 Jetty 12.0.11 - FROZEN Jun 25, 2024
lorban added a commit that referenced this issue Jun 26, 2024
@lorban
#11932 fix doc
68bc7f1 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 26, 2024
@lorban
#11932 simplify queue handling
c5873e2 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 26, 2024
@lorban
#11932 improve javadoc
d011132 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 26, 2024
@lorban @olamy
#11932 make succeeded and failed in ICB final + introduce onSuccess
032dcca 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 26, 2024
@lorban @olamy
#11932 make succeeded and failed in ICB final + introduce onSuccess
e9c2c26 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 26, 2024
@lorban @olamy
#11932 make succeeded and failed in ICB final + introduce onSuccess
b9a2308 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 26, 2024
@lorban @olamy
#11932 fix H3 message flusher
21c9111 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 26, 2024
@lorban @olamy
#11932 refactor H3 message flusher to have ICB succeeded and failed f…
fcd6eeb 
…inal

Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 26, 2024
@lorban @olamy
#11932 fix doc
8f0f4a1 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 26, 2024
@lorban @olamy
#11932 simplify queue handling
8b4078d 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 26, 2024
@lorban @olamy
#11932 improve javadoc
68e1354 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 27, 2024
@lorban @olamy
#11932 make succeeded and failed in ICB final + introduce onSuccess
d1a52cf 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 27, 2024
@lorban @olamy
#11932 make succeeded and failed in ICB final + introduce onSuccess
177e657 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 27, 2024
@lorban @olamy
#11932 make succeeded and failed in ICB final + introduce onSuccess
7e86040 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 27, 2024
@lorban @olamy
#11932 fix H3 message flusher
4bbb87d 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 27, 2024
@lorban @olamy
#11932 refactor H3 message flusher to have ICB succeeded and failed f…
34f7456 
…inal

Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 27, 2024
@lorban @olamy
#11932 fix doc
73008b7 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 27, 2024
@lorban @olamy
#11932 simplify queue handling
94ec38e 
Signed-off-by: Ludovic Orban <[email protected]>
olamy pushed a commit that referenced this issue Jun 27, 2024
@lorban @olamy
#11932 improve javadoc
83963b8 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 27, 2024
@lorban
#11932 make succeeded and failed in ICB final + introduce onSuccess
6d0bddc 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 27, 2024
@lorban
#11932 refactor H3 message flusher to have ICB succeeded and failed f…
25bbdb5 
…inal

Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 27, 2024
@lorban
#11932 simplify queue handling
d3c93bd 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 27, 2024
@lorban
#11932 improve javadoc
cea6b4a 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 27, 2024
@lorban
#11932 fix bug in case of invalid action + add test
6188757 
Signed-off-by: Ludovic Orban <[email protected]>
lorban added a commit that referenced this issue Jun 27, 2024
@lorban
#11932 revert final modifier
c0af0e6 
Signed-off-by: Ludovic Orban <[email protected]>
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in 🧊 Jetty 12.0.11 - FROZEN Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sbordet sbordet

@lorban lorban

Labels
Bug For general bugs on Jetty side
Projects
No open projects
🧊 Jetty 12.0.11 - FROZEN
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make succeeded and failed in ICB final + introduce onSuccess jetty/jetty.project
3 participants
@gregw @sbordet @lorban