Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Execution context being lost after using FutureConverters #179

Open
syzboy opened this issue Feb 26, 2020 · 5 comments
Open

Execution context being lost after using FutureConverters #179

syzboy opened this issue Feb 26, 2020 · 5 comments

Comments

@syzboy
Copy link

syzboy commented Feb 26, 2020

override def thenApply[U](fn: JF[_ >: T, _ <: U]): CompletableFuture[U] = thenApplyAsync(fn)

After converting a scala future to a java future, the synchronous functions are implemented by calling async ones.
And it's not using the ones that are taking the extra executor parameter. https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#thenApplyAsync-java.util.function.Function-

In this case, thread local values will be lost including the execution context

@payurgin
Copy link

I have the same issue with async caffeine cache (writen in java) used with scala-java8-compat.

valueFuture.whenComplete((value, error) -> {.  <---- Here we shift to ForkJoinPool.commonPool() because of scala-java8-compat
      if (!completed.compareAndSet(false, true)) {
        // Ignore multiple invocations due to ForkJoinPool retrying on delays
        return;

@rdesgroppes
Copy link

rdesgroppes commented Nov 5, 2021

Same here. How did you guys circumvent that?

@lrytz
Copy link
Member

lrytz commented Nov 5, 2021

Did any of you look at this in detail? Would it be an easy fix without unintended consequences?

@rdesgroppes
Copy link

rdesgroppes commented Nov 5, 2021

@lrytz I guess the fix would consist in taking an Executor at conversion time and propagate it down do each async call.
That said, backwards compatibility concerns would lead to either make it optional, defaulting to the current behavior, or introduce a parallel code path. Hum.

rdesgroppes added a commit to rdesgroppes/scala-java8-compat that referenced this issue Nov 5, 2021
rdesgroppes added a commit to rdesgroppes/scala-java8-compat that referenced this issue Nov 16, 2021
rdesgroppes added a commit to rdesgroppes/scala-java8-compat that referenced this issue Nov 18, 2021
@baltiyskiy
Copy link

One unfortunate feature of ForkJoinPool.commonPool() is that by default, if a Java process observes < 3 CPUs (e.g. if CPU requests in Kubernetes are set this way), then it will use a ThreadPerTaskExecutor for the common pool. This might lead to creating a lot of threads! All of this completely unexpected, because the application code doesn't specifically invoke ...async methods on CompletionStage.
Why don't these methods call the synchronous methods in the API?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants