Repository navigation
Conversation
VirtualMachineImpl.redefineClasses() removed the mirrors of the redefined types from the cache while it was building the request, that is, before the VM handled the redefinition. A thread that asked for one of those types in the meantime cached a new mirror, which could read its methods before the redefinition and then stayed in the cache. A standard HotSpot VM keeps the method IDs of a redefined class, so this had no visible effect there. A VM with enhanced class redefinition (DCEVM, e.g. the JetBrains Runtime with -XX:+AllowEnhancedClassRedefinition) gives a changed method a new ID, and reading the stack of a thread running the new code then failed with "Got MethodID of ReferenceType that is not a member of the ReferenceType" until the end of the debug session. Now the mirrors are removed from the cache again once the VM has answered the request, whether it succeeded or not. Fixes eclipse-jdt#1059 Assisted-by: Anthropic Claude (Claude Fable 5.2)
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The fix depends on concurrency timing and enhanced-redefinition VM behavior, with no automated regression coverage and DCEVM reproducer results that cannot be independently validated in the review environment.
No open issues 🎉
What changed in this PR
Addresses #1059 by clearing reference-type mirrors cached during class redefinition, preventing stale method IDs from persisting in the JDI cache.
Changes:
- Evicts affected mirrors again after the redefinition request finishes.
- Uses
finallyto perform cleanup whether the request succeeds or fails.
| File | Description |
|---|---|
| org.eclipse.jdt.debug/jdi/org/eclipse/jdi/internal/VirtualMachineImpl.java | Adds post-request cache eviction for redefined types. |
Review effort: Balanced
Author
|
This PR needs more work? If yes, what should be changed/added? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it does
Fixes #1059.
VirtualMachineImpl.redefineClasses()removed the mirrors of the redefined types from the cache before the VM handled the redefinition, so a mirror cached by another thread in the meantime could keep the method IDs from before the redefinition. Now the mirrors are removed again once the VM has answered, whether the request succeeded or not.No new automated test: the failure needs a race between two threads and a VM that changes method IDs on redefinition, which the build does not have.
Independent of #1057 (a different part of the same file, no conflict expected).
Assisted-by: Anthropic Claude (Claude Fable 5.2)
How to test
Reproducer is on issue.
Tested with the reproducer attached to the issue (100 hot code replaces per run), Eclipse 4.40 JDI with this change, Windows 10:
-XX:+AllowEnhancedClassRedefinition-XX:+AllowEnhancedClassRedefinitionAuthor checklist