0

From loom-lab, given the code

var virtualThreadFactory = Thread.ofVirtual().factory();

try (var executorService = Executors.newThreadPerTaskExecutor(virtualThreadFactory)) {
    IntStream.range(0, 15).forEach(item -> {
        executorService.submit(() -> {
            try {
                var milliseconds = item * 1000;
                System.out.println(Thread.currentThread() + " sleeping " + milliseconds + " milliseconds");
                Thread.sleep(milliseconds);
                System.out.println(Thread.currentThread() + " awake");
                if (item == 8) throw new RuntimeException("task 8 is acting up");
            } catch (InterruptedException e) {
                System.out.println("Interrupted task = " + item + ", Thread ID = " + Thread.currentThread());
            }
        });
    });
}
catch (RuntimeException e) {
    System.err.println(e.getMessage());
}

My hope was that the code would catch the RuntimeException and print the message, but it does not.

Am I hoping for too much, or will this someday work as I hope?


In response to an amazing answer by Stephen C, which I can fully appreciate, upon further exploration I discovered via

static String spawn(
        ExecutorService executorService,
        Callable<String> callable,
        Consumer<Future<String>> consumer
) throws Exception {

    try {
        var result = executorService.submit(callable);
        consumer.accept(result);
        return result.get(3, TimeUnit.SECONDS);
    }
    catch (TimeoutException e) {
        // The timeout expired...
        return callable.call() + " - TimeoutException";
    }
    catch (ExecutionException e) {
        // Why doesn't malcontent get caught here?
        return callable.call() + " - ExecutionException";

    }
    catch (CancellationException e) {   // future.cancel(false);
        // Exception was thrown
        return callable.call() + " - CancellationException";

    }
    catch (InterruptedException e) {    // future.cancel(true);
        return callable.call() + "- InterruptedException ";

    }
}

and

try (var executorService = Executors.newThreadPerTaskExecutor(threadFactory)) {
    
    Callable<String> malcontent = () -> {
        Thread.sleep(Duration.ofSeconds(2));
        throw new IllegalStateException("malcontent acting up");
    };

    System.out.println("\n\nresult = " + spawn(executorService, malcontent, (future) -> {}));

} catch (Exception e) {
    e.printStackTrace();   // malcontent gets caught here
}

I was expecting malcontent to get caught in spawn as an ExecutionException per the documentation, but it does not. Consequently, I have trouble reasoning about my expectations.

Much of my hope for Project Loom was that, unlike Functional Reactive Programming, I could once again rely on Exceptions to do the right thing, and reason about them such that I could predict what would happen without having to run experiments to validate what really happens.

As Steve Jobs (at NeXT) used to say: "It just works"

So far, my posting on loom-dev@openjdk.java.net has not been responded to... which is why I have used StackOverflow. I don't know the best way to engage the Project Loom developers.

Eric Kolotyluk
  • 1,958
  • 2
  • 21
  • 30

3 Answers3

2

This is speculation ... but I don't think so.


According to the provisional javadocs, ExecutorService now inherits AutoClosable, and it is specified that the default behavior of the close() method is to perform a clean shutdown and wait for it to complete. (Note that this is described as default behavior not required behavior!)

So why couldn't they change the behavior to catch an resignal the exceptions on this thread's stack?

One problem is that specifying patterns of behavior that are logically consistent for both this case, and the case where the ExecutorService is not used as a resource in a try with resources. In order to implement the behavior in this case, the close() method has to be informed by some other part of the executor service of the task's unhandled exception. But if nothing calls close() then the exceptions can't be re-raised. And if the close() is called in a finalizer or similar, there probably won't be anything to handle them. At the very least, it is complicated.

A second problem is that it would be difficult to handle the exception(s) in the general case. What if more than one task failed with an exception? What if different tasks failed with different exceptions? How does the code that handles the exception (e.g. your catch (RuntimeException e) ... figure out which task failed?

A third problem is that this would be a breaking change. In Java 17 and earlier, the above code would not propagate any exceptions from the tasks. In Java 18 and later it would. Java 17 code that assumed there were no "random" exceptions from failed tasks delivered to this thread would break.

A fourth point is that this would be an nuisance in use-cases where the Java 18+ programmer wants to treat the executor service as a resource, but does not want to deal with "stray" exceptions on this thread. (I suspect that would be the majority of use-cases for autoclosing an executor service.)

A fifth problem (if you want to call it that) is that it is a breaking change for early adopters of Loom. (I am reading your question as saying that you tried it with Loom and it currently doesn't behave as you proposed.)

The final problem is that there are already ways to capture a task's exception and deliver it; e.g. via the Future objects returned when you submit a task. This proposal is not filling a gap in ExecutorService functionality.

(Phew!)


Of course I don't know that the Java developers will actually do. And we won't collectively know until Loom is finally released as a non-preview feature of mainstream Java.

Anyhow, if you want to lobby for this, you should email the Loom mailing list about it.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • Wow, great answer. I have followed up by adding to my original post... I thought that `close()` is handled in the `finally` block, so even though the `try` caught something, then `close()` would still be called... so why does it care about exceptions? But I see your point about multiple exceptions, as `try` was never really designed for that... I will have to think more about the behavior I want as a developer... – Eric Kolotyluk Oct 31 '21 at 16:30
  • 1
    You don’t need `CompletableFuture`. The `ExecutorService` always returned a `Future` when you call `submit`. The underlying `Future` implementation catches all exceptions thrown by the `Callable` or `Runnable`, so its `get` method can report them. This implies that from the `ExecutorService` implementation’s point of view, there are no exceptions. It invoked the `run()` method on a `Runnable` (which is actually the same object implementing `Future` described above) and this method did not throw any exceptions. Therefore, `close()`/`shutdown()` have nothing to report to the caller. – Holger Dec 01 '21 at 13:57
  • My mistake ... `submit` returns `Future` objects. But the OP is arguing that exceptions from the tasks *should* be delivered back to the thread that manages the `ExecutorService` ... which implies that it would necessary to change a lot of things so that `close()`/ `shutdown()` *did* have something to report. My answer tries to argue that this is impractical for a number of reasons. – Stephen C Dec 01 '21 at 14:17
  • @Holger - Eric seems "miffed" that the LOOM team have not responded directly to his posting on the developer list. – Stephen C Dec 01 '21 at 14:21
  • 1
    I’m not sure whether the OP did understand the way the `ExecutorService` works in this regard, in other words, that virtual threads are entirely unrelated to this mechanism. So, whether impractical or not, such a change wasn’t even considered by the Loom team. The developer list is a different issue, it seems to full of unanswered questions… – Holger Dec 01 '21 at 14:34
  • We are on the same page :-) – Stephen C Dec 01 '21 at 14:37
1

LOOM has made many improvements such as making ExecutorService an AutoClosable so it simplifies coding, eliminating calls to shutdown / awaitTermination.

Your point on the expectation of neat exception handling applies to typical usage of ExecutorService in any JDK - not just the upcoming LOOM release - so IMO isn't obviously necessary to be tied in with LOOM work.

The error handling you wish for is quite easy to incorporate with any version of JDK by adding a few lines of code around code blocks that use ExecutorService:

var ex = new AtomicReference<RuntimeException>();
try {
    // add any use of ExecutorService here
    
    // eg OLD JDK style:
    // var executorService = Executors.newFixedThreadPool(5);

    try (var executorService = Executors.newThreadPerTaskExecutor(virtualThreadFactory)) {
       ...
          if (item == 8) {
              // Save exception before sending:
              ex.set(new RuntimeException("task 8 is acting up"));
              throw ex.get();
          }
       ...
    }
    // OR: not-LOOM JDK call executorService.shutdown/awaitTermination here

    // Pass on any handling problem
    if (ex.get() != null)
        throw ex.get();
}
catch (Exception e) {
    System.err.println("Exception was: "+e.getMessage());
}

Not elegant as you hope for, but works in any JDK release.

EDIT On your edited question:

You've put callable.call() as the code inside catch (ExecutionException e) { so that you've lost the first exception and malcontent raises a second exception. Add System.out.println to see the original:

 catch (ExecutionException e) {
     System.out.println(Thread.currentThread()+" ExecutionException: "+e);
     e.printStackTrace();

     // Why doesn't malcontent get caught here?
     return callable.call() + " - ExecutionException";
 }
DuncG
  • 12,137
  • 2
  • 21
  • 33
  • Awesome... thanks for clearing things up. My loom-lab project is a place for learning, and not just about Project Loom, but Java Concurrency in general now... The lessons I learn in StackOverflow I will try to explain in my project. – Eric Kolotyluk Oct 31 '21 at 21:24
1

I think, the closest to what you are trying to achieve, is

try(var executor = StructuredExecutor.open()) {
    var handler = new StructuredExecutor.ShutdownOnFailure();
    IntStream.range(0, 15).forEach(item -> {
        executor.fork(() -> {
            var milliseconds = item * 100;
            System.out.println(Thread.currentThread()
                + "sleeping " + milliseconds + " milliseconds");
            Thread.sleep(milliseconds);
            System.out.println(Thread.currentThread() + " awake");
            if(item == 8) {
                throw new RuntimeException("task 8 is acting up");
            }
            return null;
        }, handler);
    });

    executor.join();

    handler.throwIfFailed();
}
catch(InterruptedException|ExecutionException ex) {
    System.err.println("Caught in initiator thread");
    ex.printStackTrace();
}

which will run all jobs in virtual threads and generate an exception in the initiator thread when one of the jobs failed. StructuredExecutor is a new tool introduced by project Loom which allows to show the ownership of the created virtual threads to this specific job in diagnostic tools. But note that it’s close() won’t wait for the completion but rather requires the owner to do this before closing, throwing an exception if the developer failed to do so.

The behavior of classic ExecutorService implementations won’t change.

A solution for the ExecutorService would be

try(var executor = Executors.newVirtualThreadPerTaskExecutor()) {
    var jobs = executor.invokeAll(IntStream.range(0, 15).<Callable<?>>mapToObj(item ->
        () -> {
            var milliseconds = item * 100;
            System.out.println(Thread.currentThread()
                + " sleeping " + milliseconds + " milliseconds");
            Thread.sleep(milliseconds);
            System.out.println(Thread.currentThread() + " awake");
            if(item == 8) {
                throw new RuntimeException("task 8 is acting up");
            }
            return null;
        }).toList());

    for(var f: jobs) f.get();
}
catch(InterruptedException|ExecutionException ex) {
    System.err.println("Caught in initiator thread");
    ex.printStackTrace();
}

Note that while invokeAll waits for the completion of all jobs, we still need the loop calling get to enforce an ExecutionException to be thrown in the initiating thread.

Holger
  • 285,553
  • 42
  • 434
  • 765