auto_termination related cleanups

- disable_at_exit_handlers! has to be kept as noop not to break current usages
- cleanup auto_terminate handling
- remove ns_auto_terminate attribute
- deprecate auto_terminate=

Author: pitr-ch

CHANGELOG.md

@@ -1,5 +1,11 @@
 ## Current
 
+concurrent-ruby:
+
+* Concurrent.disable_at_exit_handlers! is not longer needed and was deprecated.
+* AbstractExecutorService#auto_terminate= was deprecated and has no effect. 
+  Set :auto_terminate option instead when executor is initialized.
+
 ## Release v1.1.6.pre1, edge v0.6.0.pre1 (26 Jan 2020)
 
 concurrent-ruby:

lib/concurrent-ruby-edge/concurrent/channel.rb

@@ -15,7 +15,7 @@ class Channel
     include Enumerable
 
     # NOTE: Move to global IO pool once stable
-    GOROUTINES = Concurrent::CachedThreadPool.new(auto_terminate: true)
+    GOROUTINES = Concurrent::CachedThreadPool.new
     private_constant :GOROUTINES
 
     BUFFER_TYPES = {

lib/concurrent-ruby/concurrent/configuration.rb

@@ -3,12 +3,14 @@
 require 'concurrent/errors'
 require 'concurrent/atomic/atomic_reference'
 require 'concurrent/concern/logging'
+require 'concurrent/concern/deprecation'
 require 'concurrent/executor/immediate_executor'
 require 'concurrent/executor/cached_thread_pool'
 require 'concurrent/utility/processor_counter'
 
 module Concurrent
   extend Concern::Logging
+  extend Concern::Deprecation
 
   autoload :Options, 'concurrent/options'
   autoload :TimerSet, 'concurrent/executor/timer_set'
@@ -96,21 +98,40 @@ def self.global_logger=(value)
   end
 
   # @!visibility private
-  GLOBAL_FAST_EXECUTOR = Delay.new { Concurrent.new_fast_executor(auto_terminate: true) }
+  GLOBAL_FAST_EXECUTOR = Delay.new { Concurrent.new_fast_executor }
   private_constant :GLOBAL_FAST_EXECUTOR
 
   # @!visibility private
-  GLOBAL_IO_EXECUTOR = Delay.new { Concurrent.new_io_executor(auto_terminate: true) }
+  GLOBAL_IO_EXECUTOR = Delay.new { Concurrent.new_io_executor }
   private_constant :GLOBAL_IO_EXECUTOR
 
   # @!visibility private
-  GLOBAL_TIMER_SET = Delay.new { TimerSet.new(auto_terminate: true) }
+  GLOBAL_TIMER_SET = Delay.new { TimerSet.new }
   private_constant :GLOBAL_TIMER_SET
 
   # @!visibility private
   GLOBAL_IMMEDIATE_EXECUTOR = ImmediateExecutor.new
   private_constant :GLOBAL_IMMEDIATE_EXECUTOR
 
+  # Disables AtExit handlers including pool auto-termination handlers.
+  # When disabled it will be the application programmer's responsibility
+  # to ensure that the handlers are shutdown properly prior to application
+  # exit by calling `AtExit.run` method.
+  #
+  # @note this option should be needed only because of `at_exit` ordering
+  #   issues which may arise when running some of the testing frameworks.
+  #   E.g. Minitest's test-suite runs itself in `at_exit` callback which
+  #   executes after the pools are already terminated. Then auto termination
+  #   needs to be disabled and called manually after test-suite ends.
+  # @note This method should *never* be called
+  #   from within a gem. It should *only* be used from within the main
+  #   application and even then it should be used only when necessary.
+  # @deprecated Has no effect since it is not longer needed, see https://github.com/ruby-concurrency/concurrent-ruby/pull/841.
+  #
+  def self.disable_at_exit_handlers!
+    deprecated "Method #disable_at_exit_handlers! has no effect since it is not longer needed, see https://github.com/ruby-concurrency/concurrent-ruby/pull/841."
+  end
+
   # Global thread pool optimized for short, fast *operations*.
   #
   # @return [ThreadPoolExecutor] the thread pool

lib/concurrent-ruby/concurrent/executor/abstract_executor_service.rb

@@ -1,4 +1,5 @@
 require 'concurrent/errors'
+require 'concurrent/concern/deprecation'
 require 'concurrent/executor/executor_service'
 require 'concurrent/synchronization'
 
@@ -8,6 +9,7 @@ module Concurrent
   # @!visibility private
   class AbstractExecutorService < Synchronization::LockableObject
     include ExecutorService
+    include Concern::Deprecation
 
     # The set of possible fallback policies that may be set at thread pool creation.
     FALLBACK_POLICIES = [:abort, :discard, :caller_runs].freeze
@@ -17,14 +19,14 @@ class AbstractExecutorService < Synchronization::LockableObject
 
     attr_reader :name
 
-    attr_accessor :ns_auto_terminate
-
     # Create a new thread pool.
     def initialize(opts = {}, &block)
       super(&nil)
       synchronize do
-        ns_initialize(opts, &block)
+        # TODO (pitr-ch 02-Feb-2020): check all documentation
+        @auto_terminate = opts.fetch(:auto_terminate, true)
         @name = opts.fetch(:name) if opts.key?(:name)
+        ns_initialize(opts, &block)
       end
     end
 
@@ -64,12 +66,12 @@ def shutdown?
 
     # @!macro executor_service_method_auto_terminate_question
     def auto_terminate?
-      synchronize { ns_auto_terminate? }
+      synchronize { @auto_terminate }
     end
 
     # @!macro executor_service_method_auto_terminate_setter
     def auto_terminate=(value)
-      synchronize { self.ns_auto_terminate = value }
+      deprecated "Method #auto_terminate= has no effect. Set :auto_terminate option when executor is initialized."
     end
 
     private
@@ -120,7 +122,7 @@ def ns_kill_execution
     end
 
     def ns_auto_terminate?
-      !!ns_auto_terminate
+      @auto_terminate
     end
 
   end

lib/concurrent-ruby/concurrent/executor/cached_thread_pool.rb

@@ -51,10 +51,9 @@ def initialize(opts = {})
     def ns_initialize(opts)
       super(opts)
       if Concurrent.on_jruby?
-        self.auto_terminate = opts.fetch(:auto_terminate, true)
         @max_queue          = 0
         @executor           = java.util.concurrent.Executors.newCachedThreadPool(
-            DaemonThreadFactory.new(self.auto_terminate?))
+            DaemonThreadFactory.new(ns_auto_terminate?))
         @executor.setRejectedExecutionHandler(FALLBACK_POLICY_CLASSES[@fallback_policy].new)
         @executor.setKeepAliveTime(opts.fetch(:idletime, DEFAULT_THREAD_IDLETIMEOUT), java.util.concurrent.TimeUnit::SECONDS)
       end

lib/concurrent-ruby/concurrent/executor/executor_service.rb

@@ -111,10 +111,10 @@ module Concurrent
 
   # @!macro executor_service_method_auto_terminate_setter
   #
-  #   Set the auto-terminate behavior for this executor.
   #
+  #   Set the auto-terminate behavior for this executor.
+  #   @deprecated Has no effect
   #   @param [Boolean] value The new auto-terminate value to set for this executor.
-  #
   #   @return [Boolean] `true` when auto-termination is enabled else `false`.
 
   ###################################################################

lib/concurrent-ruby/concurrent/executor/java_executor_service.rb

@@ -38,15 +38,13 @@ def wait_for_termination(timeout = nil)
 
       def shutdown
         synchronize do
-          self.ns_auto_terminate = false
           @executor.shutdown
           nil
         end
       end
 
       def kill
         synchronize do
-          self.ns_auto_terminate = false
           @executor.shutdownNow
           nil
         end

lib/concurrent-ruby/concurrent/executor/java_single_thread_executor.rb

@@ -17,12 +17,11 @@ def initialize(opts = {})
       end
 
       private
-      
+
       def ns_initialize(opts)
         @executor = java.util.concurrent.Executors.newSingleThreadExecutor
         @fallback_policy = opts.fetch(:fallback_policy, :discard)
         raise ArgumentError.new("#{@fallback_policy} is not a valid fallback policy") unless FALLBACK_POLICY_CLASSES.keys.include?(@fallback_policy)
-        self.auto_terminate = opts.fetch(:auto_terminate, true)
       end
     end
   end

lib/concurrent-ruby/concurrent/executor/java_thread_pool_executor.rb

@@ -108,15 +108,13 @@ def ns_initialize(opts)
           queue = java.util.concurrent.LinkedBlockingQueue.new(@max_queue)
         end
 
-        self.auto_terminate = opts.fetch(:auto_terminate, true)
-
         @executor = java.util.concurrent.ThreadPoolExecutor.new(
             min_length,
             max_length,
             idletime,
             java.util.concurrent.TimeUnit::SECONDS,
             queue,
-            DaemonThreadFactory.new(self.auto_terminate?),
+            DaemonThreadFactory.new(ns_auto_terminate?),
             FALLBACK_POLICY_CLASSES[@fallback_policy].new)
 
       end

lib/concurrent-ruby/concurrent/executor/ruby_executor_service.rb

@@ -27,7 +27,6 @@ def post(*args, &task)
     def shutdown
       synchronize do
         break unless running?
-        self.ns_auto_terminate = false
         stop_event.set
         ns_shutdown_execution
       end
@@ -37,7 +36,6 @@ def shutdown
     def kill
       synchronize do
         break if shutdown?
-        self.ns_auto_terminate = false
         stop_event.set
         ns_kill_execution
         stopped_event.set