Skip to content

Fix some ExecutorService leaks - #10198

Merged
merlimat merged 1 commit into
apache:masterfrom
lhotari:lh-cleanup-executors
Apr 14, 2021
Merged

Fix some ExecutorService leaks#10198
merlimat merged 1 commit into
apache:masterfrom
lhotari:lh-cleanup-executors

Conversation

@lhotari

@lhotari lhotari commented Apr 12, 2021

Copy link
Copy Markdown
Member

Motivation

Some tests don't close all ExecutorServices created in tests. There were also a few issues in production code where ExecutorServices weren't closed.

Modifications

  • consistently close ExecutorServices created in tests
  • prefer the usage of Lombok's @Cleanup("shutdownNow") for calling the .shutdownNow() method.
  • In tests, use shutdownNow() instead of shutdown() so that executors are closed asap instead of waiting for in-progress tasks to complete.

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great.
This fix is necessary also in order to run tests on JDK11.
Because since JDK10 Executors are no more finalizable and are not disposed automatically anymore

…eanup

- use ExecutorService.shutdownNow() instead of ExecutorService.shutdown() in tests
@lhotari
lhotari force-pushed the lh-cleanup-executors branch from b2eb1d6 to 7cbd1e8 Compare April 13, 2021 10:30
@sijie

sijie commented Apr 13, 2021

Copy link
Copy Markdown
Member

/pulsarbot run-failure-checks

2 similar comments
@lhotari

lhotari commented Apr 14, 2021

Copy link
Copy Markdown
Member Author

/pulsarbot run-failure-checks

@lhotari

lhotari commented Apr 14, 2021

Copy link
Copy Markdown
Member Author

/pulsarbot run-failure-checks

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.

5 participants