Skip to content

Commit 75e22d1

Browse files
committed
Fix RefCountedTest; add test debug on visitor walk in Ref
1 parent 518cdeb commit 75e22d1

File tree

2 files changed

+47
-4
lines changed

2 files changed

+47
-4
lines changed

src/java/org/apache/cassandra/utils/concurrent/Ref.java

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ public final class Ref<T> implements RefCounted<T>
104104
public static final boolean DEBUG_EVENTS_ENABLED = TEST_DEBUG_REF_EVENTS.getBoolean();
105105
static OnLeak ON_LEAK;
106106

107+
/** NOT INTENDED FOR USE OUTSIDE TESTS AND DEBUGGING */
108+
@VisibleForTesting
109+
private static boolean TEST_TRACE_ENABLED = false;
110+
107111
@Shared(scope = SIMULATION)
108112
public interface OnLeak
109113
{
@@ -125,6 +129,14 @@ public Ref(T referent, Tidy tidy)
125129
this.referent = referent;
126130
}
127131

132+
/**
133+
* Generally don't go around mutating this willy-nilly in non-test code please.
134+
*/
135+
@VisibleForTesting
136+
public static void enableTestTracing() { TEST_TRACE_ENABLED = true; }
137+
@VisibleForTesting
138+
public static void disableTestTracing() { TEST_TRACE_ENABLED = false; }
139+
128140
/**
129141
* Must be called exactly once, when the logical operation for which this Ref was created has terminated.
130142
* Failure to abide by this contract will result in an error (eventually) being reported, assuming a
@@ -608,7 +620,7 @@ void traverse(final RefCounted.Tidy rootObject)
608620
{
609621
if (Thread.currentThread().isInterrupted())
610622
throw new UncheckedInterruptedException(new InterruptedException());
611-
//If necessary fetch the next object to start tracing
623+
// If necessary fetch the next object to start tracing
612624
if (inProgress == null)
613625
inProgress = path.pollLast();
614626

@@ -625,6 +637,25 @@ void traverse(final RefCounted.Tidy rootObject)
625637
field = p.right;
626638
}
627639

640+
if (TEST_TRACE_ENABLED)
641+
{
642+
logger.debug("[Ref tracing for {}, Object: {}]", rootObject, child);
643+
if (field != null && child != null)
644+
{
645+
logger.debug(" - Visiting field '{}' of object {} -> value class {}",
646+
field.getName(),
647+
inProgress.o.getClass().getName(),
648+
child.getClass().getName());
649+
}
650+
else if (field != null)
651+
{
652+
// Field exists but the next child is null – still useful to know the path.
653+
logger.debug(" - Visiting field '{}' of object {} -> value is null",
654+
field.getName(),
655+
inProgress.o.getClass().getName());
656+
}
657+
}
658+
628659
if (child != null && visited.add(child))
629660
{
630661
path.offer(inProgress);

test/unit/org/apache/cassandra/utils/concurrent/RefCountedTest.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@
5252
import org.apache.cassandra.utils.concurrent.Ref.Visitor;
5353
import org.awaitility.Awaitility;
5454

55+
import static org.apache.cassandra.config.CassandraRelevantProperties.JAVA_VERSION;
56+
import static org.apache.cassandra.utils.JavaUtils.parseJavaVersion;
5557
import static org.assertj.core.api.Assertions.assertThat;
5658

5759
@SuppressWarnings({"unused", "unchecked", "rawtypes"})
@@ -468,10 +470,10 @@ Runnable getRunOnCloseLambdaWithThis()
468470
private Set<Ref.GlobalState> testCycles(Function<LambdaTestClass, Runnable> runOnCloseSupplier)
469471
{
470472
LambdaTestClass test = new LambdaTestClass();
471-
Runnable weakRef = runOnCloseSupplier.apply(test);
473+
Runnable supplierRef = runOnCloseSupplier.apply(test);
472474
RefCounted.Tidy tidier = new RefCounted.Tidy()
473475
{
474-
Runnable ref = weakRef;
476+
Runnable ref = supplierRef;
475477

476478
public void tidy()
477479
{
@@ -505,7 +507,17 @@ public String name()
505507
public void testCycles()
506508
{
507509
assertThat(testCycles(LambdaTestClass::getRunOnCloseLambdaWithThis)).isNotEmpty(); // sanity test
508-
assertThat(testCycles(LambdaTestClass::getRunOnCloseInner)).isNotEmpty(); // sanity test
510+
511+
// Behavior changed in JDK21; this was a sanity check to explore the way things worked but doesn't impact
512+
// the correctness of the final actual check of the test.
513+
int version = parseJavaVersion(JAVA_VERSION.getString());
514+
boolean runOnCloseInnerState = version >= 21;
515+
516+
var result = testCycles(LambdaTestClass::getRunOnCloseInner);
517+
if (runOnCloseInnerState)
518+
assertThat(result).isEmpty();
519+
else
520+
assertThat(result).isNotEmpty();
509521

510522
assertThat(testCycles(LambdaTestClass::getRunOnCloseLambda)).isEmpty();
511523
}

0 commit comments

Comments
 (0)