AsyncExecutor inherently leaks threads

[jakub-bochenski]

There seems to be no way to stop the threads created in AsyncExecutor, this affects mostly WhenFactory.createAsync()

[adrianluisgonzalez]

Fair point. That was originally added as a convenience method for tests.

I suppose having the When, Scheduler, Environment, PromiseHelper interfaces extend AutoCloseable is the correct way to go. Or what were your thoughts?

[jakub-bochenski]

Sounds good, but the question is should it call ExecutorService#shutdown() or ExecutorService#shutdownNow()?

I think it should also provide an awaitTermination() method to allow clients to wait until the thing is actually terminated.

[jakub-bochenski]

Actually, to keep the contract of AutoCloseable you'd have to wait for the termination to complete inside the close method.
Vertx for example doesn't implement AutoCloseable instead providing an async void close(Handler<AsyncResult<Void>> completionHandler)

[jakub-bochenski]

Fair point. That was originally added as a convenience method for tests.

Ironically I spotted this because our unit tests were spewing a ton of threads per each case :)

[adrianluisgonzalez]

If you really want shutdown(), shutdownNow() and awaitTermination() then it sounds like it needs a custom interface in when.java.

Any interest in taking a stab at it with a PR?

[jakub-bochenski]

Synchronous close() method is fine for my purpose, but I'm not sure if it will fit all use cases.

One thing I don't like about the close() approach is that bubbles through all those layers (When, Scheduler, Environment, PromiseHelper and Executor), when one is only concerned with closing the executor.

Maybe a better approach would be for WhenFactory.createAsync() to instead return two objects -- the When instance and a handle that let's you close it together with any backing resources. That way you don't pollute the interface and only factory methods that produce something that needs to be closed need to be affected.
As a secondary consideration -- I think typically I wouldn't want a regular bean using When to be able to close it (e.g. by mistake), that's something I'd do on application shutdown only.