Skip to content

Create a single place for all chunky thread/executors to be registered#1895

Open
NotStirred wants to merge 9 commits into
chunky-dev:masterfrom
NotStirred:fix/non_daemon_threads
Open

Create a single place for all chunky thread/executors to be registered#1895
NotStirred wants to merge 9 commits into
chunky-dev:masterfrom
NotStirred:fix/non_daemon_threads

Conversation

@NotStirred

@NotStirred NotStirred commented Jul 8, 2026

Copy link
Copy Markdown
Member

I'm not tied to any of the specifics here, but having a mechanism to wait on chunky to join its threads is very important with Bedrock.
Essentially leveldb is thread safe on reads, so I don't want to lock. If region parsers/chunk loading are active when the DB is closed it races and results in almost always a segfault/pthread lock error. The ChunkyThread#joinAll() lets the shutdown hook wait for everything before closing the db.

Closes #1893

  • Allows chunky to close threads on shutdown without a System.exit()
  • Additionally provides an api to wait on all chunky threads to be joined.

Closes chunky-dev#1893

- Allows chunky to close them on shutdown without a System.exit()
- Additionally provides an api to wait on all chunky threads to be joined.
@NotStirred
NotStirred requested review from ThatRedox and leMaik July 8, 2026 15:13
Comment on lines +101 to +102
@PluginApi
public static boolean joinAll(long timeout, @NotNull TimeUnit unit) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This shouldn't be @PublicApi imho, and maybe we could even not make it public?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For bedrock I need some mechanism that guarantees RegionParser and Scene#loadChunks has stopped before closing the DB, currently I'm using this api locally

Do you have any other ideas that could replace this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't have one yet, but coupling "joining all chunky threads" and "load chunks" directly together doesn't feel right. Maybe some event listeners or make RegionParser and loadChunks return CompletableFuture to make it explicit that they are asynchronous?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Btw. I really appreciate the effort to not leak threads anymore. 👏

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.

Most threads are marked as non-daemon

2 participants