From 4a7c5685c86d0349c98b2725ad01aa70ebcc3581 Mon Sep 17 00:00:00 2001
From: Matthew Draper <matthew@trebex.net>
Date: Thu, 6 Oct 2016 10:31:14 +1030
Subject: Close the IO from the read loop thread

IO#close and IO#read across threads don't get along so well:

After T1 enters #read and releases the GVL, T2 can call #close on the
IO, thereby both closing the fd and freeing the buffer while T1 is using
them.
---
 actioncable/lib/action_cable/connection/stream.rb            | 1 -
 actioncable/lib/action_cable/connection/stream_event_loop.rb | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/actioncable/lib/action_cable/connection/stream.rb b/actioncable/lib/action_cable/connection/stream.rb
index d66e1b4e41..e620b93845 100644
--- a/actioncable/lib/action_cable/connection/stream.rb
+++ b/actioncable/lib/action_cable/connection/stream.rb
@@ -106,7 +106,6 @@ module ActionCable
         def clean_rack_hijack
           return unless @rack_hijack_io
           @event_loop.detach(@rack_hijack_io, self)
-          @rack_hijack_io.close
           @rack_hijack_io = nil
         end
     end
diff --git a/actioncable/lib/action_cable/connection/stream_event_loop.rb b/actioncable/lib/action_cable/connection/stream_event_loop.rb
index eec24638b6..2d1af0ff9f 100644
--- a/actioncable/lib/action_cable/connection/stream_event_loop.rb
+++ b/actioncable/lib/action_cable/connection/stream_event_loop.rb
@@ -36,6 +36,7 @@ module ActionCable
         @todo << lambda do
           @nio.deregister io
           @map.delete io
+          io.close
         end
         wakeup
       end
-- 
cgit v1.2.3


From acd7ba795ed0f69691b7e6879f17fa50d7664ea0 Mon Sep 17 00:00:00 2001
From: Matthew Draper <matthew@trebex.net>
Date: Thu, 6 Oct 2016 11:41:18 +1030
Subject: Use a branch of websocket-client-simple, to work around read/close
 race

---
 Gemfile                         |  2 +-
 Gemfile.lock                    | 14 ++++++++++----
 actioncable/test/client_test.rb |  7 -------
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/Gemfile b/Gemfile
index e4d625d47c..6df626828c 100644
--- a/Gemfile
+++ b/Gemfile
@@ -72,7 +72,7 @@ group :cable do
   gem "hiredis", require: false
   gem "redis", require: false
 
-  gem "websocket-client-simple", require: false
+  gem "websocket-client-simple", github: "matthewd/websocket-client-simple", branch: "close-race", require: false
 
   gem "blade", require: false, platforms: [:ruby]
   gem "blade-sauce_labs_plugin", require: false, platforms: [:ruby]
diff --git a/Gemfile.lock b/Gemfile.lock
index 14635a8cb2..0d70b24b5c 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -29,6 +29,15 @@ GIT
     rb-inotify (0.9.7)
       ffi (>= 0.5.0)
 
+GIT
+  remote: https://github.com/matthewd/websocket-client-simple.git
+  revision: e161305f1a466b9398d86df3b1731b03362da91b
+  branch: close-race
+  specs:
+    websocket-client-simple (0.3.0)
+      event_emitter
+      websocket
+
 GIT
   remote: https://github.com/resque/resque.git
   revision: 20d885065ac19e7f7d7a982f4ed1296083db0300
@@ -350,9 +359,6 @@ GEM
       nokogiri
     wdm (0.1.1)
     websocket (1.2.3)
-    websocket-client-simple (0.3.0)
-      event_emitter
-      websocket
     websocket-driver (0.6.4)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.2)
@@ -414,7 +420,7 @@ DEPENDENCIES
   uglifier (>= 1.3.0)
   w3c_validators
   wdm (>= 0.1.0)
-  websocket-client-simple
+  websocket-client-simple!
 
 BUNDLED WITH
    1.13.2
diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb
index f6d4ab3202..db10a7ad16 100644
--- a/actioncable/test/client_test.rb
+++ b/actioncable/test/client_test.rb
@@ -21,13 +21,6 @@ WebSocket::Frame::Data.prepend Module.new {
     super
   end
 }
-
-WebSocket::Client::Simple::Client.prepend Module.new {
-  def initialize(*)
-    @socket = nil
-    super
-  end
-}
 #
 ####
 
-- 
cgit v1.2.3


From 1fe967d0c954f96476c471ee39455d8cceee3061 Mon Sep 17 00:00:00 2001
From: Matthew Draper <matthew@trebex.net>
Date: Thu, 6 Oct 2016 13:12:35 +1030
Subject: Wait for the socket to be closed asynchronously

---
 actioncable/test/connection/client_socket_test.rb | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/actioncable/test/connection/client_socket_test.rb b/actioncable/test/connection/client_socket_test.rb
index dff7fefbfb..bc3ff6a3d7 100644
--- a/actioncable/test/connection/client_socket_test.rb
+++ b/actioncable/test/connection/client_socket_test.rb
@@ -51,10 +51,12 @@ class ActionCable::Connection::ClientSocketTest < ActionCable::TestCase
       connection = open_connection
 
       client = connection.websocket.send(:websocket)
+      event = Concurrent::Event.new
       client.instance_variable_get("@stream")
         .instance_variable_get("@rack_hijack_io")
-        .expects(:close)
+        .define_singleton_method(:close) { event.set }
       connection.close
+      event.wait
     end
   end
 
@@ -63,7 +65,13 @@ class ActionCable::Connection::ClientSocketTest < ActionCable::TestCase
       env = Rack::MockRequest.env_for "/test",
         "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket",
         "HTTP_HOST" => "localhost", "HTTP_ORIGIN" => "http://rubyonrails.com"
-      env["rack.hijack"] = -> { env["rack.hijack_io"] = StringIO.new }
+      io = \
+        begin
+          Socket.pair(Socket::AF_UNIX, Socket::SOCK_STREAM, 0).first
+        rescue
+          StringIO.new
+        end
+      env["rack.hijack"] = -> { env["rack.hijack_io"] = io }
 
       Connection.new(@server, env).tap do |connection|
         connection.process
-- 
cgit v1.2.3