Skip to content

ThreadPoolExecutor#shutdown? inconsistency in JRuby and C Ruby #1041

Closed
@bensheldon

Description

@bensheldon
Contributor

In the scenario in which a ThreadPoolExecutor has received #shutdown and there are active threads/tasks, there is an inconsistency between what JRuby and C Ruby return for #shutdown?:

  • JRuby will return #shutdown? => true while there are still active threads/tasks, at the same time as #shuttingdown? => true. Java's executor service narrowly defines #isShutdown as "no new tasks will be accepted" which is distinct from #isTerminated which is "all tasks have completed following shut down".
  • The Ruby implementation does more of what my expectations are: #shutdown? => false while there are still active threads/tasks, at the same time as #shuttingdown? => true.

It's fairly easy to work around this inconsistency using executor.shutdown? && !executor.shuttingdown? to achieve the same result in both rubies for determining when there are no more threads/tasks on a shutdown executor. It would be nice if the behavior was aligned.

Activity

bensheldon

bensheldon commented on Feb 29, 2024

@bensheldon
ContributorAuthor

Another discovery here is that in J Ruby, once ThreadPoolExecutor#kill is called, it's necessary to do a wait_for_termination to achieve a fully shutdown and terminated executor.

This is now the shutdown wrapper I'm using:

# @param timeout [Numeric, nil] Seconds to wait for active threads.
#   * +nil+, the scheduler will trigger a shutdown but not wait for it to complete.
#   * +-1+, the scheduler will wait until the shutdown is complete.
#   * +0+, the scheduler will immediately shutdown and stop any threads.
#   * A positive number will wait that many seconds before stopping any remaining active threads.
def shutdown(timeout: -1)
  return if @executor.nil? || (@executor.shutdown? && !@executor.shuttingdown?)

  @executor.shutdown if @executor.running?

  if @executor.shuttingdown? && timeout # rubocop:disable Style/GuardClause
    executor_wait = timeout.negative? ? nil : timeout
    unless @executor.wait_for_termination(executor_wait)
      @executor.kill
      @executor.wait_for_termination
    end
  end
end

This is similar to the usage example given in Java:

 void shutdownAndAwaitTermination(ExecutorService pool) {
   pool.shutdown(); // Disable new tasks from being submitted
   try {
     // Wait a while for existing tasks to terminate
     if (!pool.awaitTermination(60, TimeUnit.SECONDS)) {
       pool.shutdownNow(); // Cancel currently executing tasks
       // Wait a while for tasks to respond to being cancelled
       if (!pool.awaitTermination(60, TimeUnit.SECONDS))
           System.err.println("Pool did not terminate");
     }
   } catch (InterruptedException ie) {
     // (Re-)Cancel if current thread also interrupted
     pool.shutdownNow();
     // Preserve interrupt status
     Thread.currentThread().interrupt();
   }
 }
bensheldon

bensheldon commented on Feb 29, 2024

@bensheldon
ContributorAuthor

Thanks! This line does look like the culprit:

def ns_shutdown?
@executor.isShutdown || @executor.isTerminated
end

I missed that last night because I got distracted trying to discover the need for the feature-check in #shuttingdown?:

def ns_shuttingdown?
if @executor.respond_to? :isTerminating
@executor.isTerminating
else
false
end

That was introduced 9+ years ago (I couldn't find the exact place), but I'm not sure if isTerminating/isTerminated only exist for certain implementations or versions of Java. Any idea how to check that?

bensheldon

bensheldon commented on Feb 29, 2024

@bensheldon
ContributorAuthor

Aha, it looks like just isTerminating maybe didn't exist in Java 6, but isTerminated did. ok, I think I have enough for a PR 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @bensheldon@eregon

      Issue actions

        ThreadPoolExecutor#shutdown? inconsistency in JRuby and C Ruby · Issue #1041 · ruby-concurrency/concurrent-ruby