vertx-virtual-threads-incubator icon indicating copy to clipboard operation
vertx-virtual-threads-incubator copied to clipboard

Thread interruption investigation (fixes needed)

Open doctorpangloss opened this issue 1 year ago • 2 comments

These tests show that Thread.currentThread().isInterrupted() doesn't get set to true, as expected, when a virtual thread is interrupted in Vertx.

Additionally, the DefaultVirtualThreadContextTest cannot interrupt Thread.Sleep(), but it's not clear to me if the reason is expected.

doctorpangloss avatar Dec 15 '22 17:12 doctorpangloss

thanks

vietj avatar Dec 19 '22 14:12 vietj

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.

vietj avatar Jan 05 '23 10:01 vietj