Closed context can still schedule timer

[ben1222]

Version

4.5.11

Context

Before 4.5.11, when setTimer from context of undeployed verticle, it will raise IllegalStateException, which could prevent new timer from being scheduled on the undeployed verticle.
However, with this line of change in 4.5.11, it no longer throws exception in this case and the timer could still be scheduled on this closed context:
https://github.com/eclipse-vertx/vert.x/pull/5344/files#diff-554a5d78ab4ac2cc98730e43ec55bb7f39f27fcbe22a1e6a7f4e6d7f9734b4eeR56

Do you have a reproducer?

Tried write a unit test in TimerTest:

@Test
public void testUndeployWhenSchedulingTimer() throws InterruptedException {
  waitFor(2);
  AtomicLong timer = new AtomicLong(-1);
  AtomicReference<String> deploymentID = new AtomicReference<>();
  CountDownLatch deployLatch = new CountDownLatch(2);
  CountDownLatch undeployLatch = new CountDownLatch(1);
  vertx.deployVerticle(new AbstractVerticle() {
    @Override
    public void start() {
      context.executeBlocking(() -> {
        deployLatch.countDown();
        try {
          undeployLatch.await();
        } catch (InterruptedException e) {
          // Ignore
        }
        try {
          timer.set(vertx.setTimer(10, id2 -> {
            fail("Should not be called");
          }));
        } catch (IllegalStateException e) {
          complete();
        }
        return null;
      }, true)
        .onComplete(onSuccess(v -> {}));
    }
  }, onSuccess(deployment -> {
    deploymentID.set(deployment);
    deployLatch.countDown();
  }));

  deployLatch.await();
  vertx.undeploy(deploymentID.get(), onSuccess(v -> {
    undeployLatch.countDown();
    complete();
  }));
  await();
  assertEquals(-1, timer.get());
}

It passed on 4.5.10 but failed on 4.5.11 with:

Starting test: TimerTest#testUndeployWhenSchedulingTimer 
java.lang.AssertionError: Should not be called
	at org.junit.Assert.fail(Assert.java:89)
	at io.vertx.test.core.AsyncTestBase.fail(AsyncTestBase.java:268)
	at io.vertx.core.TimerTest.access$2100(TimerTest.java:37)
	at io.vertx.core.TimerTest$6.lambda$start$0(TimerTest.java:478)
	at io.vertx.core.impl.VertxImpl$InternalTimerHandler.handle(VertxImpl.java:1092)
	at io.vertx.core.impl.VertxImpl$InternalTimerHandler.handle(VertxImpl.java:1063)
	at io.vertx.core.impl.ContextImpl.emit(ContextImpl.java:342)
	at io.vertx.core.impl.ContextImpl.emit(ContextImpl.java:335)
	at io.vertx.core.impl.ContextInternal.emit(ContextInternal.java:200)
	at io.vertx.core.impl.VertxImpl$InternalTimerHandler.run(VertxImpl.java:1081)
	at io.netty.util.concurrent.PromiseTask.runTask(PromiseTask.java:98)
	at io.netty.util.concurrent.ScheduledFutureTask.run(ScheduledFutureTask.java:156)
	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:173)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:166)
	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:472)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:569)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:829)
Unhandled exception 

Extra

I'm not even sure if it is intended to not fail the setTimer in this case...
I did not find explanation on why this exception was removed and how to better handle undeploy case after this change.

[tsegismont]

Have you found this issue in 4.5.11 only or in any other previous version? I'm asking because we have nothing in the release notes related to vert.x core

[tsegismont]

Sorry I read the wrong document

[tsegismont]

So in fact, it's not specific to setTimer, anything that registers a close hook with the closed context should trigger this behavior (I could reproduce with vertx.createHttpClient()).

[tsegismont]

@vietj can you take a look at this one, it's a consequence of #5344 and allows one to create an HttpClient or schedule a timer, ... etc that will not be properly closed