Releasing is reached directly by its test, and its javadoc names the real case
The javadoc said a losing releaseIfCurrent CAS is the call that arrives with no known session. releaseIfCurrent is only called by the reaper, always with a non-null expected, so it always has one; the null-known call is an overlapping release that finds the registry entry already gone. The same wrong claim was in a test's failure message. Releasing, enter and leave drop private so SessionManagerTest binds them at compile time. The six reflection helpers are gone, and a rename now breaks the build instead of a test run.
This commit is contained in:
@@ -368,18 +368,18 @@ public final class SessionManager implements TurnListener {
|
||||
/**
|
||||
* Depth count plus the terminal/role a mid-teardown pane belongs to, for
|
||||
* {@link #spawnedMemberRole}. The terminal/role come from whichever call into
|
||||
* {@link #releaseWindow} first knew them — a losing {@link #releaseIfCurrent} CAS has no
|
||||
* {@code known} session of its own, so it must not blank out what the winner already recorded.
|
||||
* {@link #releaseWindow} first knew them: a call that finds the registry entry already gone
|
||||
* passes a {@code null} session, and must not blank out what the first call recorded.
|
||||
*/
|
||||
private record Releasing(int depth, String terminalId, MemberRole role) {
|
||||
private static Releasing enter(Releasing prior, MemberSession known) {
|
||||
record Releasing(int depth, String terminalId, MemberRole role) {
|
||||
static Releasing enter(Releasing prior, MemberSession known) {
|
||||
int depth = (prior == null ? 0 : prior.depth()) + 1;
|
||||
String terminalId = known != null ? known.terminalId() : prior == null ? null : prior.terminalId();
|
||||
MemberRole role = known != null ? known.role() : prior == null ? null : prior.role();
|
||||
return new Releasing(depth, terminalId, role);
|
||||
}
|
||||
|
||||
private Releasing leave() {
|
||||
Releasing leave() {
|
||||
return depth <= 1 ? null : new Releasing(depth - 1, terminalId, role);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2558,78 +2558,38 @@ class SessionManagerTest {
|
||||
}
|
||||
|
||||
/**
|
||||
* {@code Releasing} is private, so its {@code enter}/{@code leave} are reached through
|
||||
* reflection. Each is called directly, never through {@link SessionManager#release} or
|
||||
* {@link SessionManager#spawnedMemberRole}, so a test naming one of them exercises only that
|
||||
* one — a regression in the other can never hide behind it.
|
||||
* {@code Releasing.enter} and {@code Releasing.leave} are called directly here, never through
|
||||
* {@link SessionManager#release} or {@link SessionManager#spawnedMemberRole}, so a test naming
|
||||
* one of them exercises only that one — a regression in the other can never hide behind it.
|
||||
*/
|
||||
private static Object releasingOf(int depth, String terminalId, MemberRole role) throws ReflectiveOperationException {
|
||||
Class<?> cls = Class.forName("dev.ltms.fleet.session.SessionManager$Releasing");
|
||||
java.lang.reflect.Constructor<?> ctor = cls.getDeclaredConstructor(int.class, String.class, MemberRole.class);
|
||||
ctor.setAccessible(true);
|
||||
return ctor.newInstance(depth, terminalId, role);
|
||||
}
|
||||
|
||||
private static Object releasingEnter(Object prior, MemberSession known) throws ReflectiveOperationException {
|
||||
Class<?> cls = Class.forName("dev.ltms.fleet.session.SessionManager$Releasing");
|
||||
java.lang.reflect.Method m = cls.getDeclaredMethod("enter", cls, MemberSession.class);
|
||||
m.setAccessible(true);
|
||||
return m.invoke(null, prior, known);
|
||||
}
|
||||
|
||||
private static Object releasingLeave(Object releasing) throws ReflectiveOperationException {
|
||||
java.lang.reflect.Method m = releasing.getClass().getDeclaredMethod("leave");
|
||||
m.setAccessible(true);
|
||||
return m.invoke(releasing);
|
||||
}
|
||||
|
||||
private static int depthOf(Object releasing) throws ReflectiveOperationException {
|
||||
java.lang.reflect.Method m = releasing.getClass().getDeclaredMethod("depth");
|
||||
m.setAccessible(true);
|
||||
return (int) m.invoke(releasing);
|
||||
}
|
||||
|
||||
private static String terminalIdOf(Object releasing) throws ReflectiveOperationException {
|
||||
java.lang.reflect.Method m = releasing.getClass().getDeclaredMethod("terminalId");
|
||||
m.setAccessible(true);
|
||||
return (String) m.invoke(releasing);
|
||||
}
|
||||
|
||||
private static MemberRole roleOf(Object releasing) throws ReflectiveOperationException {
|
||||
java.lang.reflect.Method m = releasing.getClass().getDeclaredMethod("role");
|
||||
m.setAccessible(true);
|
||||
return (MemberRole) m.invoke(releasing);
|
||||
}
|
||||
|
||||
@Test
|
||||
void releasingLeaveStepsDownADepthGreaterThanOneInsteadOfRemovingIt() throws ReflectiveOperationException {
|
||||
Object depthTwo = releasingOf(2, "term_a", MemberRole.DEV);
|
||||
void releasingLeaveStepsDownADepthGreaterThanOneInsteadOfRemovingIt() {
|
||||
SessionManager.Releasing depthTwo = new SessionManager.Releasing(2, "term_a", MemberRole.DEV);
|
||||
|
||||
Object afterLeave = releasingLeave(depthTwo);
|
||||
SessionManager.Releasing afterLeave = depthTwo.leave();
|
||||
|
||||
assertNotNull(afterLeave,
|
||||
"depth 2 means another release of the SAME pane is still mid-teardown; leave() must "
|
||||
+ "step the depth down, never remove the marker outright — removing it here "
|
||||
+ "is what a plain Set would do, and would reopen the window the still-in-"
|
||||
+ "flight release is relying on staying closed");
|
||||
assertEquals(1, depthOf(afterLeave));
|
||||
assertEquals("term_a", terminalIdOf(afterLeave));
|
||||
assertEquals(MemberRole.DEV, roleOf(afterLeave));
|
||||
assertEquals(1, afterLeave.depth());
|
||||
assertEquals("term_a", afterLeave.terminalId());
|
||||
assertEquals(MemberRole.DEV, afterLeave.role());
|
||||
}
|
||||
|
||||
@Test
|
||||
void releasingEnterPreservesThePriorTerminalWhenTheOverlappingCallHasNoSessionOfItsOwn()
|
||||
throws ReflectiveOperationException {
|
||||
Object prior = releasingOf(1, "term_a", MemberRole.DEV);
|
||||
void releasingEnterPreservesThePriorTerminalWhenTheOverlappingCallHasNoSessionOfItsOwn() {
|
||||
SessionManager.Releasing prior = new SessionManager.Releasing(1, "term_a", MemberRole.DEV);
|
||||
|
||||
Object afterEnter = releasingEnter(prior, null);
|
||||
SessionManager.Releasing afterEnter = SessionManager.Releasing.enter(prior, null);
|
||||
|
||||
assertEquals(2, depthOf(afterEnter), "depth still increments whether or not this enter knows its session");
|
||||
assertEquals("term_a", terminalIdOf(afterEnter),
|
||||
"a losing CAS (the releaseIfCurrent race fleetd #702 is about) has no session of "
|
||||
+ "its own to pass as known, and must not blank out the terminal the winning "
|
||||
assertEquals(2, afterEnter.depth(), "depth still increments whether or not this enter knows its session");
|
||||
assertEquals("term_a", afterEnter.terminalId(),
|
||||
"an overlapping release that finds the registry entry already gone has no session of "
|
||||
+ "its own to pass as known, and must not blank out the terminal the first "
|
||||
+ "call already recorded — that terminal is what spawnedMemberRole matches "
|
||||
+ "against");
|
||||
assertEquals(MemberRole.DEV, roleOf(afterEnter));
|
||||
assertEquals(MemberRole.DEV, afterEnter.role());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user