Skip to content

ARTEMIS-6241 mitigate potential races - #6688

Open
jbertram wants to merge 1 commit into
apache:mainfrom
jbertram:ARTEMIS-6241
Open

ARTEMIS-6241 mitigate potential races#6688
jbertram wants to merge 1 commit into
apache:mainfrom
jbertram:ARTEMIS-6241

Conversation

@jbertram

@jbertram jbertram commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Unify locking so sendToQueue() and the PUBACK/PUBREC/PUBREL/PUBCOMP handlers all synchronize on the same monitor (i.e. this), and move the duplicate-packet-ID check into sendToQueue() itself so the check and the cache mutation happen atomically under that lock. Also removes the now-unused lock field.

Refactor a few methods out of sendToQueue() to increase readability.

Add in-memory cache values instantly vs. using Transaction#afterStore to avoid races with other packet handling that needs to read in-memory values.

Lastly, move outgoing protocol logging closer to where the packet is put on the wire.

Static analysis must suffice to justify these changes as no test was able to induce the race(s).

@jbertram
jbertram force-pushed the ARTEMIS-6241 branch 2 times, most recently from 09a7292 to c602e17 Compare September 10, 2026 17:29
Unify locking so sendToQueue() and the PUBACK/PUBREC/PUBREL/PUBCOMP
handlers all synchronize on the same monitor (i.e. this), and move the
duplicate-packet-ID check into sendToQueue() itself so the check and the
cache mutation happen atomically under that lock. Also removes the
now-unused lock field.

Refactor a few methods out of sendToQueue() to increase readability.

Add in-memory cache values instantly vs. using Transaction#afterStore to
avoid races with other packet handling that needs to read in-memory
values.

Lastly, move outgoing protocol logging closer to where the packet is put
on the wire.

Static analysis must suffice to justify these changes as no test was
able to induce the race(s).
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.

1 participant