From 055b5a760a5aae8ce1e3c1a2abc3c0ccf2c84660 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sat, 6 Apr 2019 12:12:39 +1300
Subject: [PATCH 01/16] Better handling of `disconnect!`/`close`.

---
 .rubocop_todo.yml                        |  2 +-
 CHANGELOG.md                             |  1 +
 lib/slack/real_time/concurrency/async.rb | 17 +++++++++--------
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index d41606c2..d14a9bde 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-01-19 17:37:31 -0500 using RuboCop version 0.61.1.
+# on 2019-04-06 10:51:06 -0400 using RuboCop version 0.61.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
diff --git a/CHANGELOG.md b/CHANGELOG.md
index dc31b193..42a51ceb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ### 0.14.2 (Next)
 
 * [#256](https://github.com/slack-ruby/slack-ruby-client/pull/256): Added support for specifying signing secrets on a per-request basis via optional parameters to the `Slack::Events::Request` constructor - [@gabrielmdeal](https://github.com/gabrielmdeal).
+* [#262](https://github.com/slack-ruby/slack-ruby-client/pull/262): Better handling of `disconnect!`/`close` - [@ioquatix](https://github.com/ioquatix).
 * Your contribution here.
 
 ### 0.14.1 (2019/2/26)
diff --git a/lib/slack/real_time/concurrency/async.rb b/lib/slack/real_time/concurrency/async.rb
index 7c52a0f5..f1692a5a 100644
--- a/lib/slack/real_time/concurrency/async.rb
+++ b/lib/slack/real_time/concurrency/async.rb
@@ -43,11 +43,6 @@ def restart_async(client, new_url)
             end
           end
 
-          def disconnect!
-            super
-            @reactor.cancel
-          end
-
           def current_time
             ::Async::Clock.now
           end
@@ -57,14 +52,20 @@ def connect!
             run_loop
           end
 
+          # Send a close event and stop the reactor.
+          def disconnect!
+            @reactor.cancel
+            super
+            close
+          end
+
+          # Close the socket.
           def close
-            @closing = true
-            @driver.close if @driver
+            @socket.close if @socket
             super
           end
 
           def run_loop
-            @closing = false
             while @driver && @driver.next_event
               # $stderr.puts event.inspect
             end

From 43469832eb46c719d6145b6eac4e0dd7a188f6a2 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 09:41:40 +1200
Subject: [PATCH 02/16] Prefer explicit use of `@driver`.

---
 lib/slack/real_time/socket.rb | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/slack/real_time/socket.rb b/lib/slack/real_time/socket.rb
index 0b69a846..9c133608 100644
--- a/lib/slack/real_time/socket.rb
+++ b/lib/slack/real_time/socket.rb
@@ -18,9 +18,9 @@ def initialize(url, options = {})
       def send_data(message)
         logger.debug("#{self.class}##{__method__}") { message }
         case message
-        when Numeric then driver.text(message.to_s)
-        when String  then driver.text(message)
-        when Array   then driver.binary(message)
+        when Numeric then @driver.text(message.to_s)
+        when String  then @driver.text(message)
+        when Array   then @driver.binary(message)
         else false
         end
       end
@@ -29,21 +29,21 @@ def connect!
         return if connected?
 
         connect
-        logger.debug("#{self.class}##{__method__}") { driver.class }
+        logger.debug("#{self.class}##{__method__}") { @driver.class }
 
-        driver.on :message do
+        @driver.on :message do
           @last_message_at = current_time
         end
 
-        yield driver if block_given?
+        yield @driver if block_given?
       end
 
       def disconnect!
-        driver.close
+        @driver.close
       end
 
       def connected?
-        !driver.nil?
+        !@driver.nil?
       end
 
       def start_sync(client)

From 418a6455cc731fbf77e3561a5ab369bb6e61780b Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 09:42:00 +1200
Subject: [PATCH 03/16] `disconnect!` should also ensure the connection is
 closed after sending close event.

---
 lib/slack/real_time/socket.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/slack/real_time/socket.rb b/lib/slack/real_time/socket.rb
index 9c133608..89c5da11 100644
--- a/lib/slack/real_time/socket.rb
+++ b/lib/slack/real_time/socket.rb
@@ -40,6 +40,7 @@ def connect!
 
       def disconnect!
         @driver.close
+        self.close
       end
 
       def connected?

From 0319bfc2764f93d4000c086cbad65ef6a23b16ee Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 09:42:35 +1200
Subject: [PATCH 04/16] Don't use global timer for client ping loop.

---
 lib/slack/real_time/concurrency/async.rb | 33 +++++++++++-------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/lib/slack/real_time/concurrency/async.rb b/lib/slack/real_time/concurrency/async.rb
index f1692a5a..043891ce 100644
--- a/lib/slack/real_time/concurrency/async.rb
+++ b/lib/slack/real_time/concurrency/async.rb
@@ -5,10 +5,6 @@ module Slack
   module RealTime
     module Concurrency
       module Async
-        class Reactor < ::Async::Reactor
-          def_delegators :@timers, :cancel
-        end
-
         class Client < ::Async::WebSocket::Client
           extend ::Forwardable
           def_delegators :@driver, :on, :text, :binary, :emit
@@ -18,15 +14,23 @@ class Socket < Slack::RealTime::Socket
           attr_reader :client
 
           def start_async(client)
-            @reactor = Reactor.new
+            @reactor = ::Async::Reactor.new
+            
             Thread.new do
-              if client.run_ping?
-                @reactor.every(client.websocket_ping) do
-                  client.run_ping!
-                end
-              end
               @reactor.run do |task|
-                task.async do
+                if client.run_ping?
+                  task.async do |subtask|
+                    subtask.annotate "client keep-alive"
+                    
+                    while true
+                      subtask.sleep client.websocket_ping
+                      client.run_ping!
+                    end
+                  end
+                end
+                
+                task.async do |subtask|
+                  subtask.annotate "client run-loop"
                   client.run_loop
                 end
               end
@@ -52,13 +56,6 @@ def connect!
             run_loop
           end
 
-          # Send a close event and stop the reactor.
-          def disconnect!
-            @reactor.cancel
-            super
-            close
-          end
-
           # Close the socket.
           def close
             @socket.close if @socket

From f800262f9bab1490499e2faa14a3131d4898d242 Mon Sep 17 00:00:00 2001
From: dblock <dblock@dblock.org>
Date: Sat, 6 Apr 2019 18:44:43 -0400
Subject: [PATCH 05/16] Rubocop

---
 .rubocop_todo.yml                        |  2 +-
 lib/slack/real_time/concurrency/async.rb | 12 ++++++------
 lib/slack/real_time/socket.rb            |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index d14a9bde..d47e228d 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-04-06 10:51:06 -0400 using RuboCop version 0.61.1.
+# on 2019-04-06 18:44:21 -0400 using RuboCop version 0.61.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
diff --git a/lib/slack/real_time/concurrency/async.rb b/lib/slack/real_time/concurrency/async.rb
index 043891ce..6af5c18d 100644
--- a/lib/slack/real_time/concurrency/async.rb
+++ b/lib/slack/real_time/concurrency/async.rb
@@ -15,22 +15,22 @@ class Socket < Slack::RealTime::Socket
 
           def start_async(client)
             @reactor = ::Async::Reactor.new
-            
+
             Thread.new do
               @reactor.run do |task|
                 if client.run_ping?
                   task.async do |subtask|
-                    subtask.annotate "client keep-alive"
-                    
-                    while true
+                    subtask.annotate 'client keep-alive'
+
+                    loop do
                       subtask.sleep client.websocket_ping
                       client.run_ping!
                     end
                   end
                 end
-                
+
                 task.async do |subtask|
-                  subtask.annotate "client run-loop"
+                  subtask.annotate 'client run-loop'
                   client.run_loop
                 end
               end
diff --git a/lib/slack/real_time/socket.rb b/lib/slack/real_time/socket.rb
index 89c5da11..16c0df6e 100644
--- a/lib/slack/real_time/socket.rb
+++ b/lib/slack/real_time/socket.rb
@@ -40,7 +40,7 @@ def connect!
 
       def disconnect!
         @driver.close
-        self.close
+        close
       end
 
       def connected?

From f7729979df61d1d568bcbfe8ae038e3a7d773604 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 11:23:24 +1200
Subject: [PATCH 06/16] Don't use exceptions for internal flow control.

---
 lib/slack/real_time/client.rb | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/lib/slack/real_time/client.rb b/lib/slack/real_time/client.rb
index 180645ee..f07b0fc6 100644
--- a/lib/slack/real_time/client.rb
+++ b/lib/slack/real_time/client.rb
@@ -103,14 +103,29 @@ def run_loop
         end
       end
 
-      def run_ping!
+      # Ensure the server is running, and ping the remote server if no other messages were sent.
+      def keep_alive
         time_since_last_message = @socket.time_since_last_message
-        return if time_since_last_message < websocket_ping
-        raise Slack::RealTime::Client::ClientNotStartedError if !@socket.connected? || time_since_last_message > (websocket_ping * 2)
 
-        ping
-      rescue Slack::RealTime::Client::ClientNotStartedError
-        restart_async
+        # If the server responded within the specified time, we are okay:
+        return true if time_since_last_message < websocket_ping
+
+        # If the client is not connected or the server has not responded for a while:
+        if !@socket.connected? || time_since_last_message > (websocket_ping * 2)
+          return false
+        end
+
+        # Kick off the next ping message:
+        self.ping
+
+        return true
+      end
+
+      # Check if the remote server is responsive, and if not, restart the connection.
+      def run_ping!
+        unless self.keep_alive
+          restart_async
+        end
       end
 
       def run_ping?

From eb53436c9ee64e654bad8889980f502bc1c5f847 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 11:23:49 +1200
Subject: [PATCH 07/16] Keep Thread as far from everything else as possible.

---
 lib/slack/real_time/concurrency/async.rb | 41 ++++++++++++++----------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/lib/slack/real_time/concurrency/async.rb b/lib/slack/real_time/concurrency/async.rb
index 6af5c18d..ae8c52dd 100644
--- a/lib/slack/real_time/concurrency/async.rb
+++ b/lib/slack/real_time/concurrency/async.rb
@@ -13,25 +13,28 @@ class Client < ::Async::WebSocket::Client
         class Socket < Slack::RealTime::Socket
           attr_reader :client
 
-          def start_async(client)
-            @reactor = ::Async::Reactor.new
+          def start_sync(client)
+            start_reactor(client).wait
+          end
 
+          def start_async(client)
             Thread.new do
-              @reactor.run do |task|
-                if client.run_ping?
-                  task.async do |subtask|
-                    subtask.annotate 'client keep-alive'
-
-                    loop do
-                      subtask.sleep client.websocket_ping
-                      client.run_ping!
-                    end
-                  end
-                end
+              start_reactor(client)
+            end
+          end
+
+          def start_reactor(client)
+            Async do |task|
+              self.restart_async(client, @url)
 
+              if client.run_ping?
                 task.async do |subtask|
-                  subtask.annotate 'client run-loop'
-                  client.run_loop
+                  subtask.annotate 'client keep-alive'
+
+                  while true
+                    subtask.sleep client.websocket_ping
+                    run_ping!
+                  end
                 end
               end
             end
@@ -40,9 +43,13 @@ def start_async(client)
           def restart_async(client, new_url)
             @url = new_url
             @last_message_at = current_time
-            return unless @reactor
 
-            @reactor.async do
+            if @client_task
+              @client_task.stop
+            end
+
+            @client_task = task.async do |subtask|
+              subtask.annotate 'client run-loop'
               client.run_loop
             end
           end

From 92e80b51ef8caaac934bfbae47cc63b3adb144a1 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 14:55:59 +1200
Subject: [PATCH 08/16] Track failed examples.

---
 spec/spec_helper.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 0e27a18d..6a93dfc0 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -13,3 +13,8 @@
 Slack.configure do |config|
   config.token = ENV['SLACK_API_TOKEN']
 end
+
+RSpec.configure do |config|
+  # Enable flags like --only-failures and --next-failure
+  config.example_status_persistence_file_path = ".rspec_status"
+end

From 63abf626fc29b8d150b3c8cac36b53ebe0315546 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 14:57:41 +1200
Subject: [PATCH 09/16] Minor changes to integration specs. Ensure
 `client.stop!` is called from within client thread.

---
 spec/integration/integration_spec.rb | 53 +++++++++++++++-------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/spec/integration/integration_spec.rb b/spec/integration/integration_spec.rb
index ee3e2f50..869a811e 100644
--- a/spec/integration/integration_spec.rb
+++ b/spec/integration/integration_spec.rb
@@ -19,8 +19,6 @@
     Slack.configure do |slack|
       slack.logger = logger
     end
-
-    @queue = QueueWithTimeout.new
   end
 
   after do
@@ -29,15 +27,17 @@
 
   let(:client) { Slack::RealTime::Client.new(token: ENV['SLACK_API_TOKEN']) }
 
-  let(:queue) { @queue }
+  let!(:queue) { @queue = QueueWithTimeout.new }
 
   def start
     # starts the client and pushes an item on a queue when connected
     client.start_async do |driver|
       driver.on :open do |data|
         logger.debug "connection.on :open, data=#{data}"
-        queue.push nil
+        queue.push :opened
       end
+      
+      yield driver if block_given?
     end
   end
 
@@ -49,16 +49,16 @@ def start
     client.on :close do
       logger.info 'Disconnecting ...'
       # pushes another item to the queue when disconnected
-      queue.push nil if @queue
+      queue.push :closed
     end
   end
 
-  def start_server
-    dt = rand(2..6)
+  def start_server(&block)
+    dt = rand(10..20)
     logger.debug "#start_server, waiting #{dt} second(s)"
     sleep dt # prevent Slack 429 rate limit errors
     # start server and wait for on :open
-    @server = start
+    @server = start(&block)
     logger.debug "started #{@server}"
     queue.pop_with_timeout(5)
   end
@@ -72,22 +72,12 @@ def wait_for_server
     @queue = nil
   end
 
-  def stop_server
-    logger.debug '#stop_server'
-    client.stop!
-    logger.debug '#stop_server, stopped'
-  end
-
   after do
-    wait_for_server
+    wait_for_server # wait for :closed to be pushed on queue
     @server.join if @server.is_a?(::Thread)
   end
 
   context 'client connected' do
-    before do
-      start_server
-    end
-
     let(:channel) { "@#{client.self.id}" }
 
     it 'responds to message' do
@@ -105,13 +95,17 @@ def stop_server
         client.stop!
       end
 
+      start_server
+
       logger.debug "chat_postMessage, channel=#{channel}, message=#{message}"
       client.web_client.chat_postMessage channel: channel, text: message
     end
 
     it 'sends message' do
-      client.message(channel: channel, text: 'Hello world!')
-      client.stop!
+      start_server do
+        client.message(channel: channel, text: 'Hello world!')
+        client.stop!
+      end
     end
   end
 
@@ -127,19 +121,23 @@ def stop_server
 
   context 'with websocket_ping set' do
     before do
-      client.websocket_ping = 2
+      client.websocket_ping = 1
     end
+
     it 'sends pings' do
       @reply_to = nil
       client.on :pong do |data|
         @reply_to = data.reply_to
-        queue.push nil
+        queue.push :pong
         client.stop!
       end
+
       start_server
+
       queue.pop_with_timeout(5)
       expect(@reply_to).to be 1
     end
+
     it 'rebuilds the websocket connection when dropped' do
       @reply_to = nil
       client.on :pong do |data|
@@ -148,11 +146,13 @@ def stop_server
           client.instance_variable_get(:@socket).close
         else
           expect(@reply_to).to be 2
-          queue.push nil
+          queue.push :pong
           client.stop!
         end
       end
+
       start_server
+
       queue.pop_with_timeout(10)
       queue.pop_with_timeout(10)
     end
@@ -162,16 +162,21 @@ def stop_server
     before do
       client.websocket_ping = 0
     end
+
     it 'does not send pings' do
       @reply_to = nil
+
       client.on :pong do |data|
         @reply_to = data.reply_to
       end
+
       client.on :hello do
         client.stop!
       end
+
       start_server
       wait_for_server
+
       expect(@reply_to).to be nil
     end
   end

From b50eb49b03b3947a04c6343b32d2b56702c5208c Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 15:01:51 +1200
Subject: [PATCH 10/16] Don't blow up when trying to close the client.

---
 lib/slack/real_time/client.rb                   | 5 +++--
 lib/slack/real_time/concurrency/async.rb        | 6 +++++-
 lib/slack/real_time/concurrency/celluloid.rb    | 1 -
 lib/slack/real_time/concurrency/eventmachine.rb | 5 -----
 lib/slack/real_time/socket.rb                   | 8 +++++++-
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/lib/slack/real_time/client.rb b/lib/slack/real_time/client.rb
index f07b0fc6..03971e03 100644
--- a/lib/slack/real_time/client.rb
+++ b/lib/slack/real_time/client.rb
@@ -81,8 +81,6 @@ def config
 
       def run_loop
         @socket.connect! do |driver|
-          @callback.call(driver) if @callback
-
           driver.on :open do |event|
             logger.debug("#{self.class}##{__method__}") { event.class.name }
             open(event)
@@ -100,6 +98,9 @@ def run_loop
             close(event)
             callback(event, :closed)
           end
+
+          # This must be called last to ensure any events are registered before invoking user code.
+          @callback.call(driver) if @callback
         end
       end
 
diff --git a/lib/slack/real_time/concurrency/async.rb b/lib/slack/real_time/concurrency/async.rb
index ae8c52dd..34440900 100644
--- a/lib/slack/real_time/concurrency/async.rb
+++ b/lib/slack/real_time/concurrency/async.rb
@@ -65,8 +65,12 @@ def connect!
 
           # Close the socket.
           def close
-            @socket.close if @socket
             super
+          ensure
+            if @socket
+              @socket.close
+              @socket = nil
+            end
           end
 
           def run_loop
diff --git a/lib/slack/real_time/concurrency/celluloid.rb b/lib/slack/real_time/concurrency/celluloid.rb
index a2474076..e8c11095 100644
--- a/lib/slack/real_time/concurrency/celluloid.rb
+++ b/lib/slack/real_time/concurrency/celluloid.rb
@@ -46,7 +46,6 @@ def disconnect!
 
           def close
             @closing = true
-            driver.close if driver
             super
           end
 
diff --git a/lib/slack/real_time/concurrency/eventmachine.rb b/lib/slack/real_time/concurrency/eventmachine.rb
index 7f3140da..73e896f2 100644
--- a/lib/slack/real_time/concurrency/eventmachine.rb
+++ b/lib/slack/real_time/concurrency/eventmachine.rb
@@ -56,11 +56,6 @@ def disconnect!
             @thread = nil
           end
 
-          def close
-            driver.close if driver
-            super
-          end
-
           def send_data(message)
             logger.debug("#{self.class}##{__method__}") { message }
             driver.send(message)
diff --git a/lib/slack/real_time/socket.rb b/lib/slack/real_time/socket.rb
index 16c0df6e..0cbb0dad 100644
--- a/lib/slack/real_time/socket.rb
+++ b/lib/slack/real_time/socket.rb
@@ -38,8 +38,10 @@ def connect!
         yield @driver if block_given?
       end
 
+      # Gracefully shut down the connection.
       def disconnect!
         @driver.close
+      ensure
         close
       end
 
@@ -74,7 +76,11 @@ def current_time
       end
 
       def close
-        @driver = nil
+        # When you call `driver.emit(:close)`, it will typically end up calling `client.close` which will call `@socket.close` and end up back here. In order to break this infinite recursion, we check and set `@driver = nil` before invoking `client.close`.
+        if driver = @driver
+          @driver = nil
+          driver.emit(:close)
+        end
       end
 
       protected

From 1e7cb3ebb6a1337aa389a10e4f5395b1176a7f70 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 15:03:56 +1200
Subject: [PATCH 11/16] Better handling of restart loop.

---
 lib/slack/real_time/client.rb            |  7 ++--
 lib/slack/real_time/concurrency/async.rb | 44 +++++++++++++++++-------
 2 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/lib/slack/real_time/client.rb b/lib/slack/real_time/client.rb
index 03971e03..fed70c2e 100644
--- a/lib/slack/real_time/client.rb
+++ b/lib/slack/real_time/client.rb
@@ -106,13 +106,16 @@ def run_loop
 
       # Ensure the server is running, and ping the remote server if no other messages were sent.
       def keep_alive
+        # We can't ping the remote server if we aren't connected.
+        return false if @socket.nil? or !@socket.connected?
+
         time_since_last_message = @socket.time_since_last_message
 
         # If the server responded within the specified time, we are okay:
         return true if time_since_last_message < websocket_ping
 
-        # If the client is not connected or the server has not responded for a while:
-        if !@socket.connected? || time_since_last_message > (websocket_ping * 2)
+        # If the server has not responded for a while:
+        if time_since_last_message > (websocket_ping * 2)
           return false
         end
 
diff --git a/lib/slack/real_time/concurrency/async.rb b/lib/slack/real_time/concurrency/async.rb
index 34440900..7b89d23f 100644
--- a/lib/slack/real_time/concurrency/async.rb
+++ b/lib/slack/real_time/concurrency/async.rb
@@ -1,4 +1,5 @@
 require 'async/websocket'
+require 'async/notification'
 require 'async/clock'
 
 module Slack
@@ -25,18 +26,34 @@ def start_async(client)
 
           def start_reactor(client)
             Async do |task|
-              self.restart_async(client, @url)
+              @restart = ::Async::Notification.new
 
               if client.run_ping?
-                task.async do |subtask|
+                @ping_task = task.async do |subtask|
                   subtask.annotate 'client keep-alive'
 
-                  while true
+                  # The timer task will naturally exit after the driver is set to nil.
+                  while @restart
                     subtask.sleep client.websocket_ping
-                    run_ping!
+                    client.run_ping! if @restart
                   end
                 end
               end
+
+              while @restart
+                if @client_task
+                  @client_task.stop
+                end
+
+                @client_task = task.async do |subtask|
+                  subtask.annotate 'client run-loop'
+                  client.run_loop
+                end
+
+                @restart.wait
+              end
+
+              @ping_task.stop if @ping_task
             end
           end
 
@@ -44,14 +61,7 @@ def restart_async(client, new_url)
             @url = new_url
             @last_message_at = current_time
 
-            if @client_task
-              @client_task.stop
-            end
-
-            @client_task = task.async do |subtask|
-              subtask.annotate 'client run-loop'
-              client.run_loop
-            end
+            @restart.signal if @restart
           end
 
           def current_time
@@ -63,6 +73,16 @@ def connect!
             run_loop
           end
 
+          # Kill the restart/ping loop.
+          def disconnect!
+            super
+          ensure
+            if restart = @restart
+              @restart = nil
+              restart.signal
+            end
+          end
+
           # Close the socket.
           def close
             super

From 7deb9a443fe7f0b406ecf5bc2f233a5f54e4bef2 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Sun, 7 Apr 2019 15:10:49 +1200
Subject: [PATCH 12/16] Run specs first.

---
 Rakefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Rakefile b/Rakefile
index 8677c91f..85545a83 100644
--- a/Rakefile
+++ b/Rakefile
@@ -14,7 +14,7 @@ end
 require 'rubocop/rake_task'
 RuboCop::RakeTask.new
 
-task default: %i[rubocop spec]
+task default: %i[spec rubocop]
 
 load 'tasks/git.rake'
 load 'tasks/web.rake'

From 8407052e53b9cc977cc4b4014a2cd743e8356862 Mon Sep 17 00:00:00 2001
From: dblock <dblock@dblock.org>
Date: Sun, 7 Apr 2019 11:03:50 -0400
Subject: [PATCH 13/16] Rubocop.

---
 .rubocop_todo.yml                        | 12 ++++++++++--
 lib/slack/real_time/client.rb            | 18 ++++++++----------
 lib/slack/real_time/concurrency/async.rb |  6 ++----
 spec/integration/integration_spec.rb     |  2 +-
 spec/spec_helper.rb                      |  2 +-
 5 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index d47e228d..2235aaca 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1,11 +1,18 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-04-06 18:44:21 -0400 using RuboCop version 0.61.1.
+# on 2019-04-07 11:03:29 -0400 using RuboCop version 0.61.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
+# Offense count: 2
+# Configuration parameters: AllowSafeAssignment.
+Lint/AssignmentInCondition:
+  Exclude:
+    - 'lib/slack/real_time/concurrency/async.rb'
+    - 'lib/slack/real_time/socket.rb'
+
 # Offense count: 4
 Lint/HandleExceptions:
   Exclude:
@@ -45,10 +52,11 @@ Style/AccessModifierDeclarations:
 Style/GlobalVars:
   Enabled: false
 
-# Offense count: 2
+# Offense count: 3
 # Configuration parameters: MinBodyLength.
 Style/GuardClause:
   Exclude:
+    - 'lib/slack/real_time/socket.rb'
     - 'lib/slack/real_time/stores/store.rb'
     - 'lib/slack/web/faraday/response/raise_error.rb'
 
diff --git a/lib/slack/real_time/client.rb b/lib/slack/real_time/client.rb
index fed70c2e..0558fb48 100644
--- a/lib/slack/real_time/client.rb
+++ b/lib/slack/real_time/client.rb
@@ -105,9 +105,9 @@ def run_loop
       end
 
       # Ensure the server is running, and ping the remote server if no other messages were sent.
-      def keep_alive
+      def keep_alive?
         # We can't ping the remote server if we aren't connected.
-        return false if @socket.nil? or !@socket.connected?
+        return false if @socket.nil? || !@socket.connected?
 
         time_since_last_message = @socket.time_since_last_message
 
@@ -115,21 +115,19 @@ def keep_alive
         return true if time_since_last_message < websocket_ping
 
         # If the server has not responded for a while:
-        if time_since_last_message > (websocket_ping * 2)
-          return false
-        end
+        return false if time_since_last_message > (websocket_ping * 2)
 
         # Kick off the next ping message:
-        self.ping
+        ping
 
-        return true
+        true
       end
 
       # Check if the remote server is responsive, and if not, restart the connection.
       def run_ping!
-        unless self.keep_alive
-          restart_async
-        end
+        return if keep_alive?
+
+        restart_async
       end
 
       def run_ping?
diff --git a/lib/slack/real_time/concurrency/async.rb b/lib/slack/real_time/concurrency/async.rb
index 7b89d23f..25346218 100644
--- a/lib/slack/real_time/concurrency/async.rb
+++ b/lib/slack/real_time/concurrency/async.rb
@@ -41,9 +41,7 @@ def start_reactor(client)
               end
 
               while @restart
-                if @client_task
-                  @client_task.stop
-                end
+                @client_task.stop if @client_task
 
                 @client_task = task.async do |subtask|
                   subtask.annotate 'client run-loop'
@@ -57,7 +55,7 @@ def start_reactor(client)
             end
           end
 
-          def restart_async(client, new_url)
+          def restart_async(_client, new_url)
             @url = new_url
             @last_message_at = current_time
 
diff --git a/spec/integration/integration_spec.rb b/spec/integration/integration_spec.rb
index 869a811e..eb9fd85a 100644
--- a/spec/integration/integration_spec.rb
+++ b/spec/integration/integration_spec.rb
@@ -36,7 +36,7 @@ def start
         logger.debug "connection.on :open, data=#{data}"
         queue.push :opened
       end
-      
+
       yield driver if block_given?
     end
   end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 6a93dfc0..7d84b3aa 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -16,5 +16,5 @@
 
 RSpec.configure do |config|
   # Enable flags like --only-failures and --next-failure
-  config.example_status_persistence_file_path = ".rspec_status"
+  config.example_status_persistence_file_path = '.rspec_status'
 end

From da457b9b8a60efcbfbcc58ccf4f12144d6eccd37 Mon Sep 17 00:00:00 2001
From: dblock <dblock@dblock.org>
Date: Sun, 7 Apr 2019 11:07:07 -0400
Subject: [PATCH 14/16] Added .rspec_status to .gitignore.

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 75ae64de..aa1a0c66 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,4 @@ Gemfile.lock
 .DS_Store
 .bundle
 .idea
+.rspec_status

From 3a9df0112c4526938189ae1cdbe6217af4178a24 Mon Sep 17 00:00:00 2001
From: dblock <dblock@dblock.org>
Date: Mon, 8 Apr 2019 06:50:07 -0400
Subject: [PATCH 15/16] Fixed concurrency specs.

---
 spec/slack/real_time/concurrency/celluloid_spec.rb    | 5 +++++
 spec/slack/real_time/concurrency/eventmachine_spec.rb | 1 +
 2 files changed, 6 insertions(+)

diff --git a/spec/slack/real_time/concurrency/celluloid_spec.rb b/spec/slack/real_time/concurrency/celluloid_spec.rb
index 151a7b7f..4d6f3180 100644
--- a/spec/slack/real_time/concurrency/celluloid_spec.rb
+++ b/spec/slack/real_time/concurrency/celluloid_spec.rb
@@ -78,12 +78,17 @@ def read
 
           describe '#disconnect!' do
             it 'closes and nils the websocket' do
+              expect(ws).to receive(:emit)
               expect(ws).to receive(:close)
               socket.disconnect!
             end
           end
 
           context 'consumes data' do
+            let(:tcp_socket) { double(::Celluloid::IO::SSLSocket, connect: true) }
+            before do
+              allow_any_instance_of(described_class).to receive(:build_socket).and_return(tcp_socket)
+            end
             it 'runs' do
               expect(ws).to receive(:emit)
               expect(ws).to receive(:start)
diff --git a/spec/slack/real_time/concurrency/eventmachine_spec.rb b/spec/slack/real_time/concurrency/eventmachine_spec.rb
index 5de34fa6..5bfae302 100644
--- a/spec/slack/real_time/concurrency/eventmachine_spec.rb
+++ b/spec/slack/real_time/concurrency/eventmachine_spec.rb
@@ -27,6 +27,7 @@
       describe '#disconnect!' do
         it 'closes and nils the websocket' do
           socket.instance_variable_set('@driver', ws)
+          expect(ws).to receive(:emit).with(:close)
           expect(ws).to receive(:close)
           socket.disconnect!
         end

From cf6d60162277ed5cbde33e2f967c56df7548ed8e Mon Sep 17 00:00:00 2001
From: dblock <dblock@dblock.org>
Date: Mon, 8 Apr 2019 07:06:33 -0400
Subject: [PATCH 16/16] Reduce logger noise.

---
 CHANGELOG.md                             | 2 +-
 lib/slack/real_time/concurrency/async.rb | 9 +++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 42a51ceb..9e99ef28 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,7 +1,7 @@
 ### 0.14.2 (Next)
 
 * [#256](https://github.com/slack-ruby/slack-ruby-client/pull/256): Added support for specifying signing secrets on a per-request basis via optional parameters to the `Slack::Events::Request` constructor - [@gabrielmdeal](https://github.com/gabrielmdeal).
-* [#262](https://github.com/slack-ruby/slack-ruby-client/pull/262): Better handling of `disconnect!`/`close` - [@ioquatix](https://github.com/ioquatix).
+* [#257](https://github.com/slack-ruby/slack-ruby-client/pull/257), [#262](https://github.com/slack-ruby/slack-ruby-client/pull/262): Fixed occasional failures to reconnect - [@ioquatix](https://github.com/ioquatix), [@dblock](https://github.com/dblock).
 * Your contribution here.
 
 ### 0.14.1 (2019/2/26)
diff --git a/lib/slack/real_time/concurrency/async.rb b/lib/slack/real_time/concurrency/async.rb
index 25346218..d37fe790 100644
--- a/lib/slack/real_time/concurrency/async.rb
+++ b/lib/slack/real_time/concurrency/async.rb
@@ -44,8 +44,13 @@ def start_reactor(client)
                 @client_task.stop if @client_task
 
                 @client_task = task.async do |subtask|
-                  subtask.annotate 'client run-loop'
-                  client.run_loop
+                  begin
+                    subtask.annotate 'client run-loop'
+                    client.run_loop
+                  rescue ::Async::Wrapper::Cancelled => e
+                    # Will get restarted by ping worker.
+                    client.logger.warn(subtask.to_s) { e.message }
+                  end
                 end
 
                 @restart.wait