Skip to content

Commit f7075cb

Browse files
runningcodeclaude
andcommitted
perf(core): Harden lazy lock init and mark AutoClosableReentrantLock internal (JAVA-588)
Replace the unreachable candidate fallback after a failed CAS with an explicit non-null check, so a broken invariant fails loudly instead of handing two threads different locks. Mark the class @ApiStatus.Internal and make the lazy-allocation test assert the lock field directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent bd12359 commit f7075cb

2 files changed

Lines changed: 13 additions & 5 deletions

File tree

sentry/src/main/java/io/sentry/util/AutoClosableReentrantLock.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import io.sentry.ISentryLifecycleToken;
44
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
55
import java.util.concurrent.locks.ReentrantLock;
6+
import org.jetbrains.annotations.ApiStatus;
67
import org.jetbrains.annotations.NotNull;
78
import org.jetbrains.annotations.Nullable;
89
import org.jetbrains.annotations.TestOnly;
@@ -17,6 +18,7 @@
1718
* was pure GC and main-thread overhead. We keep a {@link ReentrantLock} rather than reverting to
1819
* {@code synchronized} to stay friendly to virtual threads (Loom), see #3715.
1920
*/
21+
@ApiStatus.Internal
2022
public final class AutoClosableReentrantLock {
2123

2224
private static final @NotNull AtomicReferenceFieldUpdater<
@@ -38,14 +40,13 @@ public final class AutoClosableReentrantLock {
3840
if (existing != null) {
3941
return existing;
4042
}
41-
// The loser of the race discards its candidate and uses the winner's lock, so all callers
42-
// contend on the same instance.
4343
final @NotNull ReentrantLock candidate = new ReentrantLock();
4444
if (LOCK_UPDATER.compareAndSet(this, null, candidate)) {
4545
return candidate;
4646
}
47-
final @Nullable ReentrantLock winner = lock;
48-
return winner != null ? winner : candidate;
47+
// The CAS can only fail because another thread installed its lock first, and the field is
48+
// never reset, so all callers end up contending on that same instance.
49+
return Objects.requireNonNull(lock, "lock must have been set by the winning thread");
4950
}
5051

5152
@TestOnly
@@ -54,6 +55,11 @@ boolean isLocked() {
5455
return current != null && current.isLocked();
5556
}
5657

58+
@TestOnly
59+
boolean isLockAllocated() {
60+
return lock != null;
61+
}
62+
5763
static final class AutoClosableReentrantLockLifecycleToken implements ISentryLifecycleToken {
5864

5965
private final @NotNull ReentrantLock lock;

sentry/src/test/java/io/sentry/util/AutoClosableReentrantLockTest.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ class AutoClosableReentrantLockTest {
1919
@Test
2020
fun `does not allocate the underlying lock until first acquire`() {
2121
val lock = AutoClosableReentrantLock()
22-
assertFalse(lock.isLocked)
22+
assertFalse(lock.isLockAllocated)
23+
lock.acquire().use {}
24+
assertTrue(lock.isLockAllocated)
2325
}
2426

2527
@Test

0 commit comments

Comments
 (0)