Create a single place for all chunky thread/executors to be registered#1895
Create a single place for all chunky thread/executors to be registered#1895NotStirred wants to merge 9 commits into
Conversation
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.
| @PluginApi | ||
| public static boolean joinAll(long timeout, @NotNull TimeUnit unit) { |
There was a problem hiding this comment.
This shouldn't be @PublicApi imho, and maybe we could even not make it public?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Btw. I really appreciate the effort to not leak threads anymore. 👏
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 lockerror. TheChunkyThread#joinAll()lets the shutdown hook wait for everything before closing the db.Closes #1893