Merge pull request #26714 from matthewd/close-race

Work around read/close race (x2)

6 files changed, 22 insertions, 15 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
@@ -30,6 +30,15 @@ GIT
       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
   specs:
@@ -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/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
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
-}
 #
 ####
 
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