Skip to content

Make wide flag thread-local in Utility.codeToString#502

Merged
garydgregory merged 2 commits into
apache:masterfrom
rootvector2:wide-flag-thread-local
Jun 16, 2026
Merged

Make wide flag thread-local in Utility.codeToString#502
garydgregory merged 2 commits into
apache:masterfrom
rootvector2:wide-flag-thread-local

Conversation

@rootvector2

Copy link
Copy Markdown
Contributor

Utility.codeToString keeps the WIDE-prefix state in a mutable static boolean wide, so the flag is shared by every caller. Disassembling a WIDE opcode on one thread sets it, and a different thread decoding its next local-variable instruction (iload, istore, ret, iinc, ...) then reads a 2-byte index instead of 1 and misparses the rest of that method's code. The generic parser already avoids this by keeping wide a local in Instruction.readInstruction, and the adjacent CONSUMER_CHARS signature-parser state in this same class was likewise moved to a ThreadLocal. This does the same for wide and updates the five read/clear/set sites.

Found by comparing the two wide handlers. The added UtilityTest regression reproduces it deterministically: a WIDE disassembled on one thread makes an iload on another thread decode %258 instead of %1, and it fails without the change.

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body.

shared static wide bleeds between threads disassembling code, making one thread decode the next local-variable index as 2 bytes instead of 1; keep it per-thread like the adjacent CONSUMER_CHARS.
@garydgregory garydgregory changed the title make wide flag thread-local in Utility.codeToString Make wide flag thread-local in Utility.codeToString Jun 15, 2026

@garydgregory garydgregory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hello @rootvector2

Thank you for the PR.

Please see my comments.

try {
Utility.codeToString(new ByteSequence(new byte[] {(byte) Const.WIDE}), new ConstantPool(), false);
} catch (final Exception e) {
throw new AssertionError(e);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could call org.junit.jupiter.api.Assertions.fail(String, Throwable) here.

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.

done, switched both catch blocks to fail(String, Throwable).

// iload with a single index byte; the trailing 0x02 must stay unread.
reader.set(Utility.codeToString(new ByteSequence(new byte[] {(byte) Const.ILOAD, 1, 2}), new ConstantPool(), false));
} catch (final Exception e) {
throw new AssertionError(e);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could call org.junit.jupiter.api.Assertions.fail(String, Throwable) here.

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.

done here too.

@garydgregory garydgregory merged commit 6f76438 into apache:master Jun 16, 2026
19 checks passed
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.

2 participants