Skip to content

Commit d53beed

Browse files
authored
Fix reentrant BatchingExecutor execution #1708 (#3331)
Motivation: Fork-join workers can execute another submitted batch while helping a join. The nested batch hit a single-level ThreadLocal requirement and could drop queued work. Modification: Preserve and restore the previous batch context around Batch and BlockableBatch execution. Add deterministic coverage for both modes and document nested fork-join execution. Result: Reentrant batches run without failing, and outer queued tasks continue. Normal execution retains one ThreadLocal lookup with no added allocations or locking. Tests: - sbt +actor-tests / Test / testOnly org.apache.pekko.dispatch.ExecutionContextSpec - passed on Scala 2.13 and Scala 3 - sbt headerCreateAll +headerCheckAll checkCodeStyle docs / paradox - passed - sbt +mimaReportBinaryIssues - passed - scalafmt --list --mode diff-ref=origin/main - passed - git diff --check - passed - sbt validatePullRequest - stopped at user request before completion - Qoder review - No must-fix findings References: Fixes #1708
1 parent 246fae4 commit d53beed

3 files changed

Lines changed: 44 additions & 5 deletions

File tree

actor-tests/src/test/scala/org/apache/pekko/dispatch/ExecutionContextSpec.scala

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,38 @@ class ExecutionContextSpec extends PekkoSpec(ExecutionContextSpec.config) with D
9696
Await.result(p.future, timeout.duration) should ===(())
9797
}
9898

99+
"continue the outer batch after reentrant batch execution" in {
100+
Seq(true, false).foreach { resubmit =>
101+
val submitted = new java.util.ArrayDeque[Runnable]()
102+
val executor = new BatchingExecutor {
103+
override protected def unbatchedExecute(runnable: Runnable): Unit = submitted.add(runnable)
104+
override protected def resubmitOnBlock: Boolean = resubmit
105+
override def batchable(runnable: Runnable): Boolean = true
106+
}
107+
val completed = new AtomicInteger(0)
108+
var innerBatch: Runnable = null
109+
110+
executor.execute(new Runnable {
111+
override def run(): Unit = {
112+
executor.execute(new Runnable {
113+
override def run(): Unit = completed.incrementAndGet()
114+
})
115+
// Model a pool worker executing another submitted batch while helping a join.
116+
innerBatch.run()
117+
completed.incrementAndGet()
118+
}
119+
})
120+
val outerBatch = submitted.remove()
121+
executor.execute(new Runnable {
122+
override def run(): Unit = completed.incrementAndGet()
123+
})
124+
innerBatch = submitted.remove()
125+
126+
outerBatch.run()
127+
completed.get should ===(3)
128+
}
129+
}
130+
99131
"be able to avoid starvation when Batching is used and Await/blocking is called" in {
100132
implicit val dispatcher: ExecutionContextExecutor = batchingDispatcher
101133

actor/src/main/scala/org/apache/pekko/dispatch/BatchingExecutor.scala

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ private[pekko] trait Batchable extends Runnable {
6464
@InternalApi
6565
private[pekko] trait BatchingExecutor extends Executor {
6666

67-
// invariant: if "_tasksLocal.get ne null" then we are inside Batch.run; if it is null, we are outside
67+
// _tasksLocal points to the batch accepting nested tasks, or is null when batching is inactive or paused for blocking
6868
private val _tasksLocal = new ThreadLocal[AbstractBatch]()
6969

7070
private abstract class AbstractBatch extends java.util.ArrayDeque[Runnable](4) with Runnable {
@@ -86,14 +86,17 @@ private[pekko] trait BatchingExecutor extends Executor {
8686

8787
private final class Batch extends AbstractBatch {
8888
override final def run(): Unit = {
89-
require(_tasksLocal.get eq null)
89+
val previousBatch = _tasksLocal.get
9090
_tasksLocal.set(this) // Install ourselves as the current batch
9191
try processBatch(this)
9292
catch {
9393
case t: Throwable =>
9494
resubmitUnbatched()
9595
throw t
96-
} finally _tasksLocal.remove()
96+
} finally {
97+
if (previousBatch eq null) _tasksLocal.remove()
98+
else _tasksLocal.set(previousBatch)
99+
}
97100
}
98101
}
99102

@@ -102,7 +105,7 @@ private[pekko] trait BatchingExecutor extends Executor {
102105
private final class BlockableBatch extends AbstractBatch with BlockContext {
103106
// this method runs in the delegate ExecutionContext's thread
104107
override final def run(): Unit = {
105-
require(_tasksLocal.get eq null)
108+
val previousBatch = _tasksLocal.get
106109
_tasksLocal.set(this) // Install ourselves as the current batch
107110
val firstInvocation = _blockContext.get eq null
108111
if (firstInvocation) _blockContext.set(BlockContext.current)
@@ -113,7 +116,8 @@ private[pekko] trait BatchingExecutor extends Executor {
113116
resubmitUnbatched()
114117
throw t
115118
} finally {
116-
_tasksLocal.remove()
119+
if (previousBatch eq null) _tasksLocal.remove()
120+
else _tasksLocal.set(previousBatch)
117121
if (firstInvocation) _blockContext.remove()
118122
}
119123
}

docs/src/main/paradox/typed/dispatchers.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ page describes how to use dispatchers with `pekko-actor-typed`, which has depend
2424
A Pekko `MessageDispatcher` is what makes Pekko Actors "tick", it is the engine of the machine so to speak.
2525
All `MessageDispatcher` implementations are also an @scala[`ExecutionContext`]@java[`Executor`], which means that they can be used
2626
to execute arbitrary code, for instance @scala[`Future`s]@java[`CompletableFuture`s].
27+
On fork-join dispatchers, a worker waiting for fork/join subtasks can help execute other tasks submitted to the same dispatcher.
28+
This nested execution is supported, but work that blocks on external resources should still use a dedicated dispatcher as described
29+
in @ref:[Blocking Needs Careful Management](#blocking-needs-careful-management).
2730

2831
## Default dispatcher
2932

0 commit comments

Comments
 (0)