Skip to content

Commit

Permalink
#11932 fix bug in case of invalid action + add test
Browse files Browse the repository at this point in the history
Signed-off-by: Ludovic Orban <[email protected]>
  • Loading branch information
lorban committed Jun 27, 2024
1 parent bae2e8f commit 6188757
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ private void processing()
boolean notifyCompleteSuccess = false;
Throwable notifyCompleteFailure = null;

boolean callOnSuccess = false;
// While we are processing
processing:
while (true)
Expand All @@ -260,11 +259,6 @@ private void processing()
Action action = null;
try
{
if (callOnSuccess)
{
onSuccess();
callOnSuccess = false;
}
action = process();
}
catch (Throwable x)
Expand All @@ -273,6 +267,7 @@ private void processing()
// Fall through to possibly invoke onCompleteFailure().
}

boolean callOnSuccess = false;
// acted on the action we have just received
try (AutoLock ignored = _lock.lock())
{
Expand Down Expand Up @@ -323,11 +318,11 @@ private void processing()

case CALLED:
{
callOnSuccess = true;
if (action != Action.SCHEDULED)
throw new IllegalStateException(String.format("%s[action=%s]", this, action));
// we lost the race, so we have to keep processing
_state = State.PROCESSING;
callOnSuccess = true;
continue;
}

Expand All @@ -346,6 +341,11 @@ private void processing()
throw new IllegalStateException(String.format("%s[action=%s]", this, action));
}
}
finally
{
if (callOnSuccess)
onSuccess();
}
}

if (notifyCompleteSuccess)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class IteratingCallbackTest
Expand Down Expand Up @@ -435,4 +436,28 @@ protected void onCompleteFailure(Throwable cause)

assertEquals(1, count.get());
}

@Test
public void testOnSuccessCalledDespiteISE() throws Exception
{
CountDownLatch latch = new CountDownLatch(1);
IteratingCallback icb = new IteratingCallback()
{
@Override
protected Action process()
{
succeeded();
return Action.IDLE; // illegal action
}

@Override
protected void onSuccess()
{
latch.countDown();
}
};

assertThrows(IllegalStateException.class, icb::iterate);
assertTrue(latch.await(5, TimeUnit.SECONDS));
}
}

0 comments on commit 6188757

Please sign in to comment.