Provide more ways to mark a thread as non-blocking

[trustin]

Motivation

We maintain a reactive web framework called Armeria (https://armeria.dev). It allows a user to implement a reactive web application using the Reactive Streams implementation of their choice. A user can choose from Armeria's own Reactive Streams implementation, Reactor and RxJava. Therefore, we don't have any mandatory dependency on Reactor and RxJava to keep our dependencies lean and stay unopinionated, and we want to keep it this way.

When a user writes their application using Armeria and Reactor, Armeria needs to tell Reactor that its event loop threads are non-blocking. Reactor currently determines whether a thread is non-blocking or not using Schedulers.isInNonBlockingThread() and Schedulers.isNonBlockingThread() (here).

It means, for Armeria to mark a thread as non-blocking, Armeria needs to make its event loop thread class implement the NonBlocking interface, e.g.:

public class ArmeriaEventLoopThread extends Thread implements NonBlocking { ... }

However, because Armeria has no mandatory dependency on Reactor, JVM will fail to load ArmeriaEventLoopThread when Reactor is not available in the classpath, if we introduce NonBlocking into our class hierarchy.

Desired solution

Provide an alternative way to mark a thread (or thread class) as non-blocking, so that Armeria team can define a non-blocking thread without introducing mandatory runtime dependency on reactor-core. There are a few options to solve this:

• Provide a small separate JAR that contains NonBlocking (and potentially other tagging interfaces) and nothing more.
  • Straightforward change with no additional overhead and a minimal dependency size increase.
  • However, it requires reorganization in your codebase.
  • reactor-core needs to depend on the new JAR, which may make things a little bit more complicated?
• Java SPI
  • Let a user define an SPI implementation that's loaded by Reactor in runtime.
  • This will work because: 1) our threads will not depend on any Reactor classes and 2) the SPI implementation will be loaded only when Reactor is initialized.
  • However, it will perhaps increase the overhead.
• Provide API to register a subclass of Thread as non-blocking.
  • Let a user call a new Reactor API that registers the given Thread subclass to the list of non-blocking thread types.
  • This is similar to Java SPI option, but Armeria team will need to call this API using reflection. (We're OK doing that though.)
  • Same overhead concern.
• Check the class name
  • Just check if the FQCN contains NonBlocking, as well as the instanceof NonBlocking check.
  • A little bit of overhead concern.

Considered alternatives

• As an alternative, we defined the identical duplicate NonBlocking interface in our codebase. This works because NonBlocking is just a tag interface and thus having a duplicate class in the classpath won't hurt. However, this doesn't work with Java Modules, which never allows having the same classes in more than one module.
• We could simply add reactor-core as a mandatory dependency, but our users will not be happy with it because it'd make their dependency tree bigger even if they don't use Reactor.

[trustin]

Other frameworks like Netty will benefit from resolving this issue as well, since Netty could mark their event loop thread classes as non-blocking out of the box. (but I'm not sure whether they want to introduce an additional dependency or rename their classes, so they might prefer Java SPI approach.) /cc @normanmaurer @chrisvest

[chrisvest]

A possible neutral annotation could be https://github.com/JetBrains/java-annotations/blob/master/java-annotations/src/main/java/org/jetbrains/annotations/NonBlockingExecutor.java
And the check could be that the thread, or any implemented interface, has this annotation. Would be easy to retrofit the NonBlocking interface this way.

[trustin]

Thanks for the input, @chrisvest. That's also a feasible option. Let me stay tuned to what Reactor maintainers think. Once agreed, I'd be happy to send out a PR.

[chemicL]

Hi @trustin ! I've been away for a week so here's the delayed response, thanks for your understanding :-)

Thank you for the detailed and thoughtful proposal. We discussed these options and think that a new API would be the least invasive approach. One idea would be to add the following to the Schedulers class:

private static Predicate<Thread> nonBlockingPredicate = t -> false;

public static void registerNonBlockingPredicate(Predicate<Thread> test) {
	nonBlockingPredicate = test.or(nonBlockingPredicate);
}

And the implementation of the existing methods would include that predicate:

public static boolean isInNonBlockingThread() {
	return Thread.currentThread() instanceof NonBlocking ||
		nonBlockingPredicate.test(Thread.currentThread());
}

This of course skips the necessary synchronization and the reset functionality, which would be required for testing purposes at least, but similar ideas can be seen in the Hooks class, e.g. for onErrorDroppedHook.

Regarding the check whether reactor is on the classpath, you can consider a class loader lookup, like Spring Framework checks for the context-propagation library or the way reactor-core does. It's also important to remember the separation of the ContextPropagation class that actually uses the types from the runtime library, while the ContextPropagationSupport class only fences the usages from it - that's dictated by native image preprocessing if I'm not mistaken.

For the other options, they seem a bit too advanced for a basic functionality like this one. Also, regarding @chrisvest proposal, we would prefer not to add an external runtime dependency. It seems in Netty there is a dependency on the Jetbrains annotations library, but a compile-time one (scope provided in Maven terms would mean that if I'm not mistaken). So for us it would be a hard dependency to include the annotations, while if we create a jar with the annotation it would be another dependency for Armeria. I'm not sure whether Netty would consider that either. For further context, currently, reactor-netty adds the marker interface to wrap Netty's FastThreadLocalThread. Potentially, Netty could use the new reactor-core API... with the technique described above, but most probably users of Netty will be able to leverage the new API whenever they are interacting with reactor-core.

The SPI sounds interesting, but quite heavy in terms of maintenance for a small task like this one. Also, it can lead to potential issues with shading and excluding META-INF directory (e.g. reactor-netty excludes it from Netty due to GraalVM preprocessing AFAIK) and we'd suggest to avoid that risk as debugging for it is not straightforward.

Let me know what you think, we are of course happy to accept a contribution!

[trustin]

Thanks for the reply, @chemicL. Your suggestion sounds good to me. Let me try to find some time to make a contribution. 🙇

[chemicL]

Resolved by #3854. Thank you, @trustin .