On Mon, 8 Apr 2024 09:20:44 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
> The RejectedExecutionException was thrown when the thread executing > `Server.start` managed to shut down the `compilerThreadPool` before the > thread executing `Server.handleRequest` submitted the compilation task. > > This patch removes the extra thread used for `Server.handleRequest`, and > executes that method directly in the thread pool. All `compilerThreadPool` > uses happen on the `Server.start` thread now, and no new tasks are submitted > after the thread pool is shut down. > > In order to verify the fix, I modified `IdleMonitor.KEEPALIVE` to 1 second. > With that change the problem was occasionally reproducible without the patch > from this PR. With the patch, the `RejectedExecutionException` problem did > not reproduce. > > No new regression test. Existing langtools tests continue to pass. My understanding of the existing model is that we wanted to read the command on the socket without delay and then potentially wait until there was a free executor thread in the pool. With the new model, we won't read the input on the socket until there is a free thread. I don't know if this could ever be a problem, but it seems plausible. The default size of the thread pool is number of CPUs in the system. The default number of jobs in the makefiles is never larger than this number, so in reality, we won't get more compile requests than number of CPUs from the makefiles when using default values. However, if the user manually increases the JOBS number in the makefiles, we could. Perhaps it would be worth at least trying running make with a much larger JOBS value with this patch to see that it doesn't fall apart? ------------- PR Review: https://git.openjdk.org/jdk/pull/18672#pullrequestreview-1986545820
