[
https://issues.apache.org/jira/browse/SLING-13298?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Nageshwari Elango updated SLING-13298:
--------------------------------------
Description:
h2. Problem
In {+}OakMockSlingRepository.deactivate(){+}, all three executors are
terminated *before* the Oak JCR repository is shut down:
{code:java}
// Current order in deactivate() — verified from source
executor.shutdownNow();
scheduledExecutor.shutdownNow();
shutdownExecutorService(repository, "scheduledExecutor"); // reflects into
repository object
((JackrabbitRepository) repository).shutdown();
{code}
When +repository.shutdown()+ is subsequently called, it triggers
+BackgroundObserver.close()+ on registered observers. +close()+ itself is
straightforward — it clears the queue and adds a STOP sentinel:
{code:java}
// BackgroundObserver.close() — verified from source
public synchronized void close() {
queue.clear();
queue.add(STOP);
stopped = true;
}
{code}
The problem is not in +close()+ itself. The problem arises when a task that was
*already in-flight* at shutdown time completes. Its completion handler is
registered via {+}currentTask.onComplete(completionHandler){+}, and that
handler does:
{code:java}
// completionHandler.run() — verified from source
public void run() {
currentTask = new NotifyingFutureTask(task);
executor.execute(currentTask); // throws RejectedExecutionException —
executor already dead
}
{code}
Because all executors have already been terminated via +shutdownNow()+ before
+repository.shutdown()+ is called, every such chained task submission throws
{+}RejectedExecutionException{+}:
{noformat}
Task org.apache.jackrabbit.oak.commons.concurrent.NotifyingFutureTask@1748c62
[Not completed, task =
org.apache.jackrabbit.oak.spi.commit.BackgroundObserver$1$1@54b0813]
rejected from java.util.concurrent.ThreadPoolExecutor@2545113e
[Shutting down, pool size = 1, active threads = 1, queued tasks = 0, completed
tasks = 0]
{noformat}
h2. Impact
* Affects all unit tests using +ResourceResolverType.JCR_OAK+ — not specific
to acs-aem-commons.
* All tests still pass. This is teardown noise only.
* GitHub Actions surfaces these exceptions as {+}failure{+}-level CI
annotations, making them visually indistinguishable from real test failures.
Contributors must manually inspect every annotation to confirm none are genuine.
* Observed consistently on {*}Windows JDK 17 and JDK 21{*}; intermittently on
macOS/Linux (timing-dependent).
* Confirmed pre-existing across multiple runs — seen in [acs-aem-commons PR
#3760|https://github.com/Adobe-Consulting-Services/acs-aem-commons/pull/3760]
(merged June 2026), unrelated to any code change in that repo.
* No impact on production Oak or production AEM — exclusively in the test
framework teardown path.
h2. Root Cause
+OakMockSlingRepository+ creates the executors and passes them into Oak via
+Oak.with(executor)+ and {+}Jcr.with(executor){+}. The owner should outlive its
dependents during shutdown. Currently it does not: all three executors are
force-killed before +repository.shutdown()+ is called, so any Oak background
task still in-flight at that moment fails to chain its next step.
h2. Proposed Fix
Call +repository.shutdown()+ first, while the executors are still alive, then
terminate them:
{code:java}
// Proposed order
// Shut down Oak JCR repository first so any in-flight BackgroundObserver
// tasks can complete their chaining before the executor is terminated
((JackrabbitRepository) repository).shutdown();
executor.shutdownNow();
scheduledExecutor.shutdownNow();
shutdownExecutorService(repository, "scheduledExecutor");
{code}
The existing intent — force-kill executors without waiting, since this is
unit-test context — is preserved. Only the ordering changes.
*Points for maintainers to evaluate:*
* Whether reversing the order alone is sufficient, or whether a short
+awaitTermination()+ between +repository.shutdown()+ and
+executor.shutdownNow()+ is needed to allow in-flight chained tasks to complete
cleanly before the executor is killed.
* Whether any Oak internal components beyond +BackgroundObserver+ depend on
these executors during {+}repository.shutdown(){+}.
h2. Steps to Reproduce
# Add a dependency on +sling-mock-oak+ and write a test using
{+}ResourceResolverType.JCR_OAK{+}.
# Run the test suite.
# Observe +RejectedExecutionException+ from
+org.apache.jackrabbit.oak.spi.commit.BackgroundObserver+ in teardown output.
Example project:
[acs-aem-commons|https://github.com/Adobe-Consulting-Services/acs-aem-commons]
— [CI run
#30953046297|https://github.com/enageshwari/acs-aem-commons/actions/runs/30953046297]
h2. Source References
*
[OakMockSlingRepository.java|https://github.com/apache/sling-org-apache-sling-testing-sling-mock-oak/blob/master/src/main/java/org/apache/sling/testing/mock/sling/oak/OakMockSlingRepository.java]
*
[BackgroundObserver.java|https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-spi/src/main/java/org/apache/jackrabbit/oak/spi/commit/BackgroundObserver.java]
was:
----
h1. Summary
sling-mock-oak: OakMockSlingRepository.deactivate() terminates executors before
repository.shutdown(), causing RejectedExecutionException from
BackgroundObserver's completion handler during test teardown
----
h1. Description
h2. Problem
In {+}OakMockSlingRepository.deactivate(){+}, all three executors are
terminated *before* the Oak JCR repository is shut down:
{code:java}
// Current order in deactivate() — verified from source
executor.shutdownNow();
scheduledExecutor.shutdownNow();
shutdownExecutorService(repository, "scheduledExecutor"); // reflects into
repository object
((JackrabbitRepository) repository).shutdown();
{code}
When +repository.shutdown()+ is subsequently called, it triggers
+BackgroundObserver.close()+ on registered observers. +close()+ itself is
straightforward — it clears the queue and adds a STOP sentinel:
{code:java}
// BackgroundObserver.close() — verified from source
public synchronized void close() {
queue.clear();
queue.add(STOP);
stopped = true;
}
{code}
The problem is not in +close()+ itself. The problem arises when a task that was
*already in-flight* at shutdown time completes. Its completion handler is
registered via {+}currentTask.onComplete(completionHandler){+}, and that
handler does:
{code:java}
// completionHandler.run() — verified from source
public void run() {
currentTask = new NotifyingFutureTask(task);
executor.execute(currentTask); // throws RejectedExecutionException —
executor already dead
}
{code}
Because all executors have already been terminated via +shutdownNow()+ before
+repository.shutdown()+ is called, every such chained task submission throws
{+}RejectedExecutionException{+}:
{noformat}
Task org.apache.jackrabbit.oak.commons.concurrent.NotifyingFutureTask@1748c62
[Not completed, task =
org.apache.jackrabbit.oak.spi.commit.BackgroundObserver$1$1@54b0813]
rejected from java.util.concurrent.ThreadPoolExecutor@2545113e
[Shutting down, pool size = 1, active threads = 1, queued tasks = 0, completed
tasks = 0]
{noformat}
h2. Impact
* Affects all unit tests using +ResourceResolverType.JCR_OAK+ — not specific
to acs-aem-commons.
* All tests still pass. This is teardown noise only.
* GitHub Actions surfaces these exceptions as {+}failure{+}-level CI
annotations, making them visually indistinguishable from real test failures.
Contributors must manually inspect every annotation to confirm none are genuine.
* Observed consistently on {*}Windows JDK 17 and JDK 21{*}; intermittently on
macOS/Linux (timing-dependent).
* Confirmed pre-existing across multiple runs — seen in [acs-aem-commons PR
#3760|https://github.com/Adobe-Consulting-Services/acs-aem-commons/pull/3760]
(merged June 2026), unrelated to any code change in that repo.
* No impact on production Oak or production AEM — exclusively in the test
framework teardown path.
h2. Root Cause
+OakMockSlingRepository+ creates the executors and passes them into Oak via
+Oak.with(executor)+ and {+}Jcr.with(executor){+}. The owner should outlive its
dependents during shutdown. Currently it does not: all three executors are
force-killed before +repository.shutdown()+ is called, so any Oak background
task still in-flight at that moment fails to chain its next step.
h2. Proposed Fix
Call +repository.shutdown()+ first, while the executors are still alive, then
terminate them:
{code:java}
// Proposed order
// Shut down Oak JCR repository first so any in-flight BackgroundObserver
// tasks can complete their chaining before the executor is terminated
((JackrabbitRepository) repository).shutdown();
executor.shutdownNow();
scheduledExecutor.shutdownNow();
shutdownExecutorService(repository, "scheduledExecutor");
{code}
The existing intent — force-kill executors without waiting, since this is
unit-test context — is preserved. Only the ordering changes.
*Points for maintainers to evaluate:*
* Whether reversing the order alone is sufficient, or whether a short
+awaitTermination()+ between +repository.shutdown()+ and
+executor.shutdownNow()+ is needed to allow in-flight chained tasks to complete
cleanly before the executor is killed.
* Whether any Oak internal components beyond +BackgroundObserver+ depend on
these executors during {+}repository.shutdown(){+}.
h2. Steps to Reproduce
# Add a dependency on +sling-mock-oak+ and write a test using
{+}ResourceResolverType.JCR_OAK{+}.
# Run the test suite.
# Observe +RejectedExecutionException+ from
+org.apache.jackrabbit.oak.spi.commit.BackgroundObserver+ in teardown output.
Example project:
[acs-aem-commons|https://github.com/Adobe-Consulting-Services/acs-aem-commons]
— [CI run
#30953046297|https://github.com/enageshwari/acs-aem-commons/actions/runs/30953046297]
h2. Source References
*
[OakMockSlingRepository.java|https://github.com/apache/sling-org-apache-sling-testing-sling-mock-oak/blob/master/src/main/java/org/apache/sling/testing/mock/sling/oak/OakMockSlingRepository.java]
*
[BackgroundObserver.java|https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-spi/src/main/java/org/apache/jackrabbit/oak/spi/commit/BackgroundObserver.java]
* Related (different issue): SLING-12250
> sling-mock-oak: OakMockSlingRepository.deactivate() terminates executors
> before repository.shutdown(), causing RejectedExecutionException from
> BackgroundObserver's completion handler during test teardown
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: SLING-13298
> URL: https://issues.apache.org/jira/browse/SLING-13298
> Project: Sling
> Issue Type: Bug
> Components: Apache Sling Testing Clients
> Reporter: Nageshwari Elango
> Priority: Minor
>
> h2. Problem
> In {+}OakMockSlingRepository.deactivate(){+}, all three executors are
> terminated *before* the Oak JCR repository is shut down:
> {code:java}
> // Current order in deactivate() — verified from source
> executor.shutdownNow();
> scheduledExecutor.shutdownNow();
> shutdownExecutorService(repository, "scheduledExecutor"); // reflects into
> repository object
> ((JackrabbitRepository) repository).shutdown();
> {code}
> When +repository.shutdown()+ is subsequently called, it triggers
> +BackgroundObserver.close()+ on registered observers. +close()+ itself is
> straightforward — it clears the queue and adds a STOP sentinel:
> {code:java}
> // BackgroundObserver.close() — verified from source
> public synchronized void close() {
> queue.clear();
> queue.add(STOP);
> stopped = true;
> }
> {code}
> The problem is not in +close()+ itself. The problem arises when a task that
> was *already in-flight* at shutdown time completes. Its completion handler is
> registered via {+}currentTask.onComplete(completionHandler){+}, and that
> handler does:
> {code:java}
> // completionHandler.run() — verified from source
> public void run() {
> currentTask = new NotifyingFutureTask(task);
> executor.execute(currentTask); // throws RejectedExecutionException —
> executor already dead
> }
> {code}
> Because all executors have already been terminated via +shutdownNow()+ before
> +repository.shutdown()+ is called, every such chained task submission throws
> {+}RejectedExecutionException{+}:
> {noformat}
> Task org.apache.jackrabbit.oak.commons.concurrent.NotifyingFutureTask@1748c62
> [Not completed, task =
> org.apache.jackrabbit.oak.spi.commit.BackgroundObserver$1$1@54b0813]
> rejected from java.util.concurrent.ThreadPoolExecutor@2545113e
> [Shutting down, pool size = 1, active threads = 1, queued tasks = 0,
> completed tasks = 0]
> {noformat}
> h2. Impact
> * Affects all unit tests using +ResourceResolverType.JCR_OAK+ — not specific
> to acs-aem-commons.
> * All tests still pass. This is teardown noise only.
> * GitHub Actions surfaces these exceptions as {+}failure{+}-level CI
> annotations, making them visually indistinguishable from real test failures.
> Contributors must manually inspect every annotation to confirm none are
> genuine.
> * Observed consistently on {*}Windows JDK 17 and JDK 21{*}; intermittently
> on macOS/Linux (timing-dependent).
> * Confirmed pre-existing across multiple runs — seen in [acs-aem-commons PR
> #3760|https://github.com/Adobe-Consulting-Services/acs-aem-commons/pull/3760]
> (merged June 2026), unrelated to any code change in that repo.
> * No impact on production Oak or production AEM — exclusively in the test
> framework teardown path.
> h2. Root Cause
> +OakMockSlingRepository+ creates the executors and passes them into Oak via
> +Oak.with(executor)+ and {+}Jcr.with(executor){+}. The owner should outlive
> its dependents during shutdown. Currently it does not: all three executors
> are force-killed before +repository.shutdown()+ is called, so any Oak
> background task still in-flight at that moment fails to chain its next step.
> h2. Proposed Fix
> Call +repository.shutdown()+ first, while the executors are still alive, then
> terminate them:
> {code:java}
> // Proposed order
> // Shut down Oak JCR repository first so any in-flight BackgroundObserver
> // tasks can complete their chaining before the executor is terminated
> ((JackrabbitRepository) repository).shutdown();
> executor.shutdownNow();
> scheduledExecutor.shutdownNow();
> shutdownExecutorService(repository, "scheduledExecutor");
> {code}
> The existing intent — force-kill executors without waiting, since this is
> unit-test context — is preserved. Only the ordering changes.
> *Points for maintainers to evaluate:*
> * Whether reversing the order alone is sufficient, or whether a short
> +awaitTermination()+ between +repository.shutdown()+ and
> +executor.shutdownNow()+ is needed to allow in-flight chained tasks to
> complete cleanly before the executor is killed.
> * Whether any Oak internal components beyond +BackgroundObserver+ depend on
> these executors during {+}repository.shutdown(){+}.
> h2. Steps to Reproduce
> # Add a dependency on +sling-mock-oak+ and write a test using
> {+}ResourceResolverType.JCR_OAK{+}.
> # Run the test suite.
> # Observe +RejectedExecutionException+ from
> +org.apache.jackrabbit.oak.spi.commit.BackgroundObserver+ in teardown output.
> Example project:
> [acs-aem-commons|https://github.com/Adobe-Consulting-Services/acs-aem-commons]
> — [CI run
> #30953046297|https://github.com/enageshwari/acs-aem-commons/actions/runs/30953046297]
> h2. Source References
> *
> [OakMockSlingRepository.java|https://github.com/apache/sling-org-apache-sling-testing-sling-mock-oak/blob/master/src/main/java/org/apache/sling/testing/mock/sling/oak/OakMockSlingRepository.java]
> *
> [BackgroundObserver.java|https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-spi/src/main/java/org/apache/jackrabbit/oak/spi/commit/BackgroundObserver.java]
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
