ThreadLocals on GraphQL-Java

Question

I'm exposing a legacy web app on GraphQL, but this web app uses Threadlocals (amongst other Apache-Shiro).

Since GraphQL-java seems to be using the fork-join pool for concurrency I worry about how far I need to go to ensure that my ThreadLocals still work and work safely.

Reading the documentation and the source it seems a large part of the concurrency is achieved by DataFetchers that return CompletableFuture's I can't tell for sure if that's the only source of concurrency (i think not) and whether the DataFetchers themselves are invoked from the fork-join pool

So would it be Safe to wrap my DataFetcher's in a delegate that set and clears the ThreadLocals? or does that still have the risk of being preempted and continued on another thread in the fork-join pool something like:

static class WrappedDataFetcher implements DataFetcher<Object> {
        private DataFetcher<?> realDataFetcher;

        WrappedDataFetcher(DataFetcher<?> realDataFetcher) {
            this.realDataFetcher = realDataFetcher;
        }

        @Override
        public Object get(DataFetchingEnvironment dataFetchingEnvironment) throws Exception {
            try {
                setThreadLocalsFromRequestOrContext(dataFetchingEnvironment);
                return realDataFetcher.get(dataFetchingEnvironment);
            } finally {
                clearTreadLocals();
            }
        }
    }

Or would I need to explicitly run my DataFetchers in a Threadpool like:

    static class WrappedDataFetcherThreadPool implements DataFetcher<Object> {
        private DataFetcher<?> wrappedDataFetcher;
        private ThreadPoolExecutor executor;


        WrappedDataFetcherThreadPool(DataFetcher<?> realDataFetcher, ThreadPoolExecutor executor) {
            // Wrap in Wrapper from previous example to ensure threadlocals in the executor
            this.wrappedDataFetcher = new WrappedDataFetcher(realDataFetcher);
            this.executor = executor;
        }

        @Override
        public Object get(DataFetchingEnvironment dataFetchingEnvironment) throws Exception {
            Future<?> future = executor.submit(() -> wrappedDataFetcher.get(dataFetchingEnvironment));
            return future.get(); //for simplicity / clarity of the question
        }
    }

I think the second one solves my problem but it feels like overkill and I worry about performance. But I think the first risks preemption.

If there is a better way to handle this I would love to hear it as well.

Note: this is not about the async nature of GraphQL (I hope to leverage that as well) but about the possible side effect of running multiple requests WITH treadLocals that might get mixed up between requests due to the fork-join pool

Answer 1

As far as I know graphql-java does not use its own thread pool and relies on the application for it. The way it achieves it using future callbacks. Say this is the current state of the application.

Thread T_1 with thread local storage TLS_1 executing data fetcher DF_1.

Graphql-java engine attaches a synchronous callback to the future returned by DF_1. If a future is not returned it wraps the result in a completed future and then attaches the synchronous callback. Since the callback is synchronous the thread that completes the future runs the callback. If any other thread apart from T_1 completes the future, TLS_1 is lost(unless it's copied over to the executing thread). One example of this is a non blocking HTTP I/O library which uses an I/O thread to complete the response future.

Here is a link where the authors have commented more on the thread behavior in graphql-java library

https://spectrum.chat/graphql-java/general/how-to-supply-custom-executor-service-for-data-fetchers-to-run-on~29caa730-9114-4883-ab4a-e9700f225f93