Return the same Thread object from Thread.currentThread() - #175
Conversation
Every attached thread now owns its java/lang/Thread instance: attach takes the instance for threads started via Thread.start (so currentThread() inside run() is the started Thread object) and creates one otherwise (bootstrap, external attachers). currentThread() returns the stored instance, and the GC roots it per thread. Also parse unrecognized classfile attributes as an opaque Unknown variant instead of failing — JVMS 4.7.1 requires silently ignoring them, and the anonymous-class fixture carries EnclosingMethod and Signature attributes the parser rejected. Expected output for the fixture is generated by a real JVM.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #175 +/- ##
==========================================
+ Coverage 83.89% 84.08% +0.19%
==========================================
Files 173 173
Lines 12939 12958 +19
==========================================
+ Hits 10855 10896 +41
+ Misses 2084 2062 -22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes java.lang.Thread.currentThread() so it returns the same per-attached-thread java/lang/Thread instance instead of allocating a new object on every call, aligning identity semantics with real JVM behavior (including inside started threads). It also updates classfile parsing to tolerate unknown attributes as required by the JVMS.
Changes:
- Store a per-thread
java/lang/Threadinstance onJvmThread, and makeThread.currentThread()return that stored instance. - Update thread attach/start paths so Java-started threads attach using the started
Threadreceiver; bootstrap/external threads create an internal thread instance. - Keep unrecognized classfile attributes as an
Unknownvariant instead of failing parsing; add an E2E fixture forcurrentThread()identity expectations.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test_data/src/CurrentThread.java | Adds an E2E Java fixture validating currentThread() reference identity across calls and threads. |
| test_data/CurrentThread.txt | Adds expected output for the CurrentThread E2E fixture. |
| jvm/src/thread.rs | Extends JvmThread to store an owned java/lang/Thread instance. |
| jvm/src/jvm.rs | Makes attach_thread accept/create a per-thread Java Thread object and adds current_java_thread(). |
| jvm/src/garbage_collector.rs | Treats each thread’s stored Java Thread object as a GC root. |
| java_runtime/tests/classes/java/lang/test_object.rs | Updates test thread attach calls to the new async attach_thread(None) API. |
| java_runtime/src/classes/java/lang/thread.rs | Ensures Thread.start() attaches the spawned thread with the correct Java Thread receiver; updates currentThread() implementation. |
| classfile/src/attribute.rs | Preserves unknown attributes as AttributeInfo::Unknown instead of rejecting the classfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Judged the two remaining remote branches on the fork: - dependabot/cargo/tracing-attributes-0.1.31: deleted. PR #4 (fa92ef9) removed the tracing-attributes direct dependency outright, so the branch patches a Cargo.toml line that no longer exists. - wie-ktf-hardening: preserved. 8 of its 12 commits are already in upstream/main via squash merges (dlunch#174 dlunch#175 dlunch#176 dlunch#177 dlunch#180 dlunch#182); git cherry missed this because origin/main trails upstream/main by 20 commits. 4 commits carry residual value. No code changes. Co-authored-by: jun0 <junyoung.choi.a@miraeasset.com> Co-authored-by: Claude <noreply@anthropic.com>
…am-sync-s1-tracing-cut-1f356ae] * Bump bytemuck from 1.25.0 to 1.25.1 (dlunch#173) Bumps [bytemuck](https://github.com/Lokathor/bytemuck) from 1.25.0 to 1.25.1. - [Changelog](https://github.com/Lokathor/bytemuck/blob/main/changelog.md) - [Commits](Lokathor/bytemuck@v1.25.0...v1.25.1) --- updated-dependencies: - dependency-name: bytemuck dependency-version: 1.25.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Replace runtime panics with the matching Java exceptions (dlunch#174) * Replace runtime panics with the matching Java exceptions An unwrap audit found panics reachable from ordinary Java code: - File.length() returns 0 for a missing file; isDirectory/isFile lose their guard-then-unwrap shape - FileImpl (native runtime) maps open/read/write/seek failures to IOError instead of panicking, so FileInputStream and RandomAccessFile guards actually produce FileNotFoundException; FileOutputStream gains the same guard - File I/O operations (read/write/seek/available/length/setLength) throw java.io.IOException on failure via a shared helper - Class.forName resolves the class and throws ClassNotFoundException (new runtime class) instead of panicking on any not-yet-loaded name - StringBuffer.append(char)/append(char[]) keep exact UTF-16 units so unpaired surrogates no longer panic and pairs built char by char survive; String.valueOf(char) builds through [C for the same reason - PrintStream.println(char) replaces an unpaired surrogate with '?' like the JDK charset encoder - ZipFile validates the archive in its constructor and throws java.util.zip.ZipException (new runtime class) for a malformed archive; getInputStream returns null for a missing entry Expected outputs for the new fixtures are generated by a real JVM. Remaining unwraps are invariants (interpreter stack discipline, thread attach), guarded lookups, or documented gaps (lenient calendar normalization, ClassFormatError plumbing). * Inline the IOException conversion at each I/O call site * Return the same Thread object from Thread.currentThread() (dlunch#175) Every attached thread now owns its java/lang/Thread instance: attach takes the instance for threads started via Thread.start (so currentThread() inside run() is the started Thread object) and creates one otherwise (bootstrap, external attachers). currentThread() returns the stored instance, and the GC roots it per thread. Also parse unrecognized classfile attributes as an opaque Unknown variant instead of failing — JVMS 4.7.1 requires silently ignoring them, and the anonymous-class fixture carries EnclosingMethod and Signature attributes the parser rejected. Expected output for the fixture is generated by a real JVM. * Add Java primitive wrapper classes (dlunch#176) * Add Java primitive wrapper classes * Use Character digit semantics for numeric parsing * Bump tokio from 1.52.3 to 1.52.4 (dlunch#179) Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.52.3 to 1.52.4. - [Release notes](https://github.com/tokio-rs/tokio/releases) - [Commits](tokio-rs/tokio@tokio-1.52.3...tokio-1.52.4) --- updated-dependencies: - dependency-name: tokio dependency-version: 1.52.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [rustjava-upstream-sync-s1-tracing-cut-1f356ae] docs: record S1 landing (conflicts 2, setProperty descriptor breakage) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Inseok Lee <git@dlun.ch> Co-authored-by: jun0 <junyoung.choi.a@miraeasset.com>
Summary
Thread.currentThread()was a stub that constructed a brand-newThreadobject on every call, socurrentThread() == currentThread()wasfalse, and inside a started threadcurrentThread()had no relation to theThreadobject that was started.Changes
java/lang/Threadinstance, stored on the JVM's per-thread state.Jvm::attach_threadnow takes the instance for threads started from Java (Thread.startpasses its receiver, socurrentThread()insiderun()is the started object) and creates one otherwise (bootstrap/main thread, external attachers).Thread.currentThread()returns the stored instance; missing is an invariant violation.Threadobject as a root;detach_threaddrops it with the thread state.Unknownvariant instead of failing the parse — JVMS 4.7.1 requires unrecognized attributes to be silently ignored. Found because the anonymous-class fixture carriesEnclosingMethod/Signatureattributes the parser rejected outright.Test plan
CurrentThreadE2E (expected output from a real JVM): same-thread calls compare equal,currentThread()inside a startedRunnableis the startedThreadobject, and differs from the main thread's. Fails before the change (false/false/false); the anonymous-class fixture also failed to parse before the attribute fix.cargo test --workspacegreen, fmt clean, clippy no new warnings.