Skip to content

Commit e77b4fc

Browse files
committed
Fix vacuous flush-Error assertion in SenderPoolErrorSafetyTest
flushErrorDiscardsBrokenSenderInsteadOfRecycling asserted assertNotSame(first, second) on the two PooledSender wrappers. SenderPool.borrow() allocates a fresh PooledSender on every call, so that comparison is unconditionally true and proves nothing: it stays green whether the broken slot was discarded (discardBroken) or recycled (giveBack). Reverting PooledSender.close()'s discard fix left the test green, so the only coverage of the flush-Error discard path was dead. Capture the underlying SenderSlot before close() and compare slots via a new slotOf() reflective helper, mirroring the sibling fix in d6bdbdf (SenderPoolTest.testBrokenSenderIsNotReturnedToPool). The pool recycles slots, not wrappers, so a broken slot leaking back to the next borrower now surfaces as the same slot and fails the assertion directly. The existing "close() must propagate the Error" assertion is unchanged. Verified: with the bug injected (giveBack instead of discardBroken) the assertion now fails as expected; green against the real discardBroken.
1 parent df6f7ca commit e77b4fc

1 file changed

Lines changed: 16 additions & 1 deletion

File tree

core/src/test/java/io/questdb/client/test/impl/SenderPoolErrorSafetyTest.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@
2525
package io.questdb.client.test.impl;
2626

2727
import io.questdb.client.Sender;
28+
import io.questdb.client.impl.PooledSender;
2829
import io.questdb.client.impl.SenderPool;
2930
import org.junit.Assert;
3031
import org.junit.Test;
3132

33+
import java.lang.reflect.Field;
3234
import java.lang.reflect.Proxy;
3335
import java.nio.file.Paths;
3436
import java.util.concurrent.atomic.AtomicBoolean;
@@ -95,6 +97,13 @@ public void flushErrorDiscardsBrokenSenderInsteadOfRecycling() throws Exception
9597

9698
try (SenderPool pool = newPool(CFG, 1, 1, 1_000, factory)) {
9799
Sender first = pool.borrow();
100+
// Capture the underlying slot before close(): borrow() always hands
101+
// out a FRESH PooledSender wrapper, so assertNotSame(first, second)
102+
// on the wrappers is vacuously true and proves nothing -- it stays
103+
// true whether or not the broken slot was discarded. The pool
104+
// recycles slots, not wrappers, so a broken slot leaking back to
105+
// the next borrower shows up as the SAME slot. Assert on the slot.
106+
Object firstSlot = slotOf(first);
98107
try {
99108
first.close();
100109
Assert.fail("close() must propagate the Error thrown by flush()");
@@ -106,7 +115,7 @@ public void flushErrorDiscardsBrokenSenderInsteadOfRecycling() throws Exception
106115
try {
107116
Assert.assertNotSame(
108117
"a sender whose flush() exited with an Error must be discarded, not recycled",
109-
first, second);
118+
firstSlot, slotOf(second));
110119
} finally {
111120
// second's flush() also throws on close(); swallow on teardown.
112121
try {
@@ -118,6 +127,12 @@ public void flushErrorDiscardsBrokenSenderInsteadOfRecycling() throws Exception
118127
}
119128
}
120129

130+
private static Object slotOf(Sender pooledWrapper) throws Exception {
131+
Field f = PooledSender.class.getDeclaredField("slot");
132+
f.setAccessible(true);
133+
return f.get(pooledWrapper);
134+
}
135+
121136
// Like fakeSender(), but flush() throws an Error to drive the
122137
// PooledSender.close() abnormal-exit branch.
123138
private static Sender flushThrowingSender() {

0 commit comments

Comments
 (0)