Skip to content

Fix NPE when re-using ArrayByteBufferPool after it was cleared when the QueuedPool is used - #15808

Merged
lorban merged 14 commits into
jetty-12.1.xfrom
fix/12.1.x/15807-npe-in-compounded-ArrayByteBufferPool-clear
Sep 24, 2026
Merged

Fix NPE when re-using ArrayByteBufferPool after it was cleared when the QueuedPool is used#15808
lorban merged 14 commits into
jetty-12.1.xfrom
fix/12.1.x/15807-npe-in-compounded-ArrayByteBufferPool-clear

Conversation

@lorban

@lorban lorban commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #15807

@lorban
#15807 fix NPE when re-using ABBP after it was cleared when the Queue…
73a0edd 
…dPool is used

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
lorban requested a review from sbordet September 17, 2026 09:50
@lorban lorban self-assigned this Sep 17, 2026
@lorban lorban added the Bug For general bugs on Jetty side label Sep 17, 2026
@lorban lorban moved this to 👀 In review in Jetty 12.1.14 FROZEN Sep 17, 2026
@lorban lorban mentioned this pull request Sep 17, 2026
Closed
@lorban
fix checkstyle
7229d07 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
sbordet
sbordet requested changes Sep 18, 2026
View reviewed changes
Comment thread jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/QueuedPool.java Outdated
@lorban
remove an entry from the queue only once per Entry.remove() call, and…
4dbcff1 
… update the queueSize

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
lorban requested a review from sbordet September 18, 2026 06:50
@lorban
fix checkstyle
2e70ccf 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
fix checkstyle
915bfcb 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
sbordet
sbordet requested changes Sep 18, 2026
View reviewed changes
Comment thread jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/QueuedPool.java Outdated

private void remove(QueuedEntry<P> entry)
{
if (queue.remove(entry))

@sbordet sbordet Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is right WRT the comment in QueuedPool.terminate().
Either we change this, or change that comment if it's ok to concurrently remove entries while terminate() runs.

@lorban lorban Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I've now revised remove() and terminate() to better align how they work and I added an extra test asserting they stay coherent.

@lorban
align Entry.remove() with QueuedPool.terminate()
62412a4 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
lorban requested a review from sbordet September 18, 2026 08:34
sbordet
sbordet requested changes Sep 18, 2026
View reviewed changes
Comment thread jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/QueuedPool.java Outdated
Comment thread jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/QueuedPool.java Outdated
@lorban
handle review comments
d35562b 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@sbordet
sbordet self-requested a review September 21, 2026 07:38
@lorban
QueuedEntry CAS should not be looped over as a failure always indicat…
5fbc22a 
…e that the action can never succeed

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
fix queueSize counter coherency w.r.t pool size
ffb3990 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
rework QueuedPool.QueuedEntry so that the pooled field's value never …
59f9b0c 
…needs to be nulled to enact a state change

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
fix bug where the leaked entry might have moved to another position i…
5ef86b2 
…n the list

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
sbordet
sbordet requested changes Sep 22, 2026
View reviewed changes
Comment thread jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/QueuedPool.java
Comment thread jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ConcurrentPool.java Outdated
@lorban
fix double-counting of leaked entries when two concurrent threads det…
ec46669 
…ect one and try to remove it

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
fix the state of terminated entries and do not ask the pool to remove…
eafc623 
… known out-of-pool entries to avoid scanning the queue in the common case

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
lorban requested a review from sbordet September 23, 2026 10:19
sbordet
sbordet requested changes Sep 23, 2026
View reviewed changes
Comment thread jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/QueuedPool.java Outdated
@lorban
clear the queue before calling QueuedEntry.remove() so the latter sca…
ea8341e 
…ns an empty queue per remove() call

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban lorban mentioned this pull request Sep 24, 2026
Merged
@lorban
lorban merged commit 0b571ef into jetty-12.1.x Sep 24, 2026
11 checks passed
@github-project-automation github-project-automation Bot moved this from 👀 In review to ✅ Done in Jetty 12.1.14 FROZEN Sep 24, 2026
@lorban
lorban deleted the fix/12.1.x/15807-npe-in-compounded-ArrayByteBufferPool-clear branch September 24, 2026 07:32
@3dgiordano 3dgiordano mentioned this pull request Sep 24, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sbordet sbordet sbordet approved these changes

Assignees

@lorban lorban

Labels

Bug For general bugs on Jetty side

Projects

Jetty 12.1.14 FROZEN
Status: ✅ Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ArrayByteBufferPool.clear() leaves the bucket's secondary QueuedPool poisoned: NPE on the next acquire()

2 participants

@lorban @sbordet