Open doctorpangloss opened 1 year ago
thanks
thanks for this interesting PR, after examination here is what I found
First the thread interruption is not correct because in some case (the default scheduler), it will not be executed because the sleep operation will not yield the task execution to the current context, instead it should be done from another plain thread, e.g
new Thread(() -> {
Thread.State st;
while ((st = thisThread.getState()) != Thread.State.WAITING && st != Thread.State.TIMED_WAITING) {
try {
Thread.sleep(1);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
thisThread.interrupt();
}
}).start();
Then the interrupted check is not correct in some tests, because Thread.sleep
clears the interrupted status (as per the javadoc).
If I do change both then tests are passing.
NOTE: Thread.sleep
will not allow executing other tasks on the context with the default scheduler but it does with the event-loop scheduler. We could remediate this with by adding a sleep method on Async that lets the scheduler be aware of this before performing the actual thread sleep.
These tests show that
Thread.currentThread().isInterrupted()
doesn't get set totrue
, as expected, when a virtual thread is interrupted in Vertx.Additionally, the
DefaultVirtualThreadContextTest
cannot interruptThread.Sleep()
, but it's not clear to me if the reason is expected.