Skip to content

Shut down a directory poller's thread pool when the poller stops - #613

Open
JakeHuneau wants to merge 1 commit into
OpenAS2:masterfrom
JakeHuneau:fix/poller-thread-pool-leak
Open

JakeHuneau wants to merge 1 commit into
OpenAS2:masterfrom
JakeHuneau:fix/poller-thread-pool-leak

Conversation

@JakeHuneau

Copy link
Copy Markdown
Contributor

A directory poller running in parallel mode leaks its thread pool every time the partnerships are reloaded.

Cause

DirectoryPollingModule.init creates a fixed thread pool when process_files_in_parallel is set:

if (processFilesAsThreads) {
    executorService = Executors.newFixedThreadPool(maxProcessingThreads);
}

Nothing shuts it down. PollingModule.doStop only cancels the timer, DirectoryPollingModule has no destroy, and BaseSession.destroyPartnershipPollers calls stop() rather than destroy() in any case.

Partnership pollers are destroyed and rebuilt on every partnerships reload, because refreshConfig calls destroyPartnershipPollers and then recreates them. The module object is dropped but the pool's threads are still alive, and live threads are garbage collection roots, so the pool and everything it references survives for the life of the process. Every reload leaks another one, up to max_parallel_files threads per poller.

Fix

Stopping a poller shuts its pool down.

The shutdown refuses new work but lets a file that is part way through being sent finish, rather than interrupting it: an interrupted transmission would leave the partner with an incomplete message and this side unsure whether it arrived. If that work has not drained within ten seconds the wait gives up and logs rather than blocking the reload, and the threads end when the work completes.

Starting a poller again now gives it a usable pool. Without that the restart would leave the poller holding the pool that had just been shut down under it, and a shut down pool rejects everything handed to it, so the poller would have looked alive while failing every file it picked up. That is a hazard the fix introduces if only the shutdown half is done, so both halves are here.

Scope

Only deployments with process_files_in_parallel enabled were affected, as no pool is created otherwise.

Testing

184 tests pass, 4 new. The test runs a real server with parallel mode enabled and takes the poller from the session. Mutation checks: removing the shutdown fails 2 tests, removing the restart guard fails 2.

  • aRunningPollerHasAUsablePool
  • stoppingThePollerShutsItsPoolDown
  • aPollerThatIsStartedAgainGetsAWorkingPool
  • theOutboxIsStillPolledAfterARestart

A directory poller running in parallel mode creates a fixed thread pool in
init and nothing ever shut it down. Stopping a poller only cancelled its
timer, and destroy is never called on one.

Partnership pollers are destroyed and rebuilt every time the partnerships are
reloaded, which happens whenever the partnerships file changes. The module
object was dropped but its pool's threads stayed alive, and live threads are
garbage collection roots, so the pool and everything it referenced survived
for the life of the process. Every reload leaked another one.

Stopping a poller now shuts its pool down. The shutdown refuses new work but
lets a file that is part way through being sent finish, because interrupting a
transmission would leave the partner with an incomplete message and this side
unsure whether it arrived. If that work has not drained within ten seconds the
wait gives up and says so rather than blocking the reload, and the threads end
when the work completes.

Starting a poller again now gives it a usable pool. Without that, the restart
would leave it holding the pool that had just been shut down under it, and a
shut down pool rejects everything handed to it, so the poller would have looked
alive while failing every file it picked up.

Only deployments with "process_files_in_parallel" enabled were affected, since
no pool is created otherwise.
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

Successfully merging this pull request may close these issues.

1 participant