From 0583076c2b1795eedd5313864e2786876b9974e4 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Thu, 7 Apr 2022 19:56:34 +0200
Subject: [PATCH] Fix Thread leakage when scoring or testing submissions

It is discouraged to do anything directly within the
Tubesock hijack block. We might only use the callbacks
(onopen, onmessage, onclose, onerror). Otherwise,
Tubesock might not close all Threads correctly and
will keep them awake every five seconds.
---
 Gemfile                                   |  2 +-
 Gemfile.lock                              | 24 +++++++------------
 app/controllers/submissions_controller.rb | 29 ++++++++++++++---------
 3 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/Gemfile b/Gemfile
index 6e8adcbc..cad0b7cf 100644
--- a/Gemfile
+++ b/Gemfile
@@ -42,7 +42,7 @@ gem 'sass-rails'
 gem 'slim-rails'
 gem 'sorcery' # Causes a deprecation warning in Rails 6.0+, see: https://github.com/Sorcery/sorcery/pull/255
 gem 'telegraf'
-gem 'tubesock', github: 'gosukiwi/tubesock', branch: 'patch-1' # Switch to a fork which is compatible with Rails 5
+gem 'tubesock'
 gem 'turbolinks'
 gem 'webpacker'
 gem 'whenever', require: false
diff --git a/Gemfile.lock b/Gemfile.lock
index 39c110b1..a5356d11 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -6,15 +6,6 @@ GIT
       json (~> 2.6.1)
       structured_warnings (~> 0.4.0)
 
-GIT
-  remote: https://github.com/gosukiwi/tubesock.git
-  revision: 86a5ca4f7d3c3a7b9a727ad91df3b9b4912eda39
-  branch: patch-1
-  specs:
-    tubesock (0.2.7)
-      rack (>= 1.5.0)
-      websocket (>= 1.1.0)
-
 GIT
   remote: https://github.com/openHPI/proforma.git
   revision: dc68000325388e1d75f31be9e136a82edad8a56d
@@ -162,7 +153,7 @@ GEM
       regexp_parser (~> 2.2)
     erubi (1.10.0)
     eventmachine (1.2.7)
-    excon (0.92.1)
+    excon (0.92.2)
     factory_bot (6.2.1)
       activesupport (>= 5.0.0)
     factory_bot_rails (6.2.0)
@@ -238,7 +229,7 @@ GEM
     listen (3.7.1)
       rb-fsevent (~> 0.10, >= 0.10.3)
       rb-inotify (~> 0.9, >= 0.9.10)
-    loofah (2.15.0)
+    loofah (2.16.0)
       crass (~> 1.0.2)
       nokogiri (>= 1.5.9)
     mail (2.7.1)
@@ -264,7 +255,7 @@ GEM
     mnemosyne-ruby (1.12.1)
       activesupport (>= 4)
       bunny
-    msgpack (1.4.5)
+    msgpack (1.5.1)
     multi_json (1.15.0)
     multi_xml (0.6.0)
     nested_form (0.3.2)
@@ -287,7 +278,7 @@ GEM
       rack (>= 1.2, < 3)
     pagedown-bootstrap-rails (2.1.4)
       railties (> 3.1)
-    parallel (1.22.0)
+    parallel (1.22.1)
     parser (3.1.1.0)
       ast (~> 2.4.1)
     path_expander (1.1.0)
@@ -397,7 +388,7 @@ GEM
     rspec-expectations (3.11.0)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.11.0)
-    rspec-mocks (3.11.0)
+    rspec-mocks (3.11.1)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.11.0)
     rspec-rails (5.1.1)
@@ -495,6 +486,9 @@ GEM
     temple (0.8.2)
     thor (1.2.1)
     tilt (2.0.10)
+    tubesock (0.2.9)
+      rack (>= 1.5.0)
+      websocket (>= 1.1.0)
     turbolinks (5.2.1)
       turbolinks-source (~> 5.2)
     turbolinks-source (5.2.0)
@@ -603,7 +597,7 @@ DEPENDENCIES
   sorcery
   spring
   telegraf
-  tubesock!
+  tubesock
   turbolinks
   web-console
   webmock
diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb
index c1e6418b..83fefdb4 100644
--- a/app/controllers/submissions_controller.rb
+++ b/app/controllers/submissions_controller.rb
@@ -85,7 +85,10 @@ class SubmissionsController < ApplicationController
 
     hijack do |tubesock|
       client_socket = tubesock
-      return kill_client_socket(client_socket) if @embed_options[:disable_run]
+
+      client_socket.onopen do |_event|
+        kill_client_socket(client_socket) if @embed_options[:disable_run]
+      end
 
       client_socket.onclose do |_event|
         runner_socket&.close(:terminated_by_client)
@@ -177,16 +180,18 @@ class SubmissionsController < ApplicationController
 
   def score
     hijack do |tubesock|
-      return if @embed_options[:disable_score]
+      tubesock.onopen do |_event|
+        kill_client_socket(tubesock) if @embed_options[:disable_score]
 
-      tubesock.send_data(JSON.dump(@submission.calculate_score))
-      # To enable hints when scoring a submission, uncomment the next line:
-      # send_hints(tubesock, StructuredError.where(submission: @submission))
+        tubesock.send_data(JSON.dump(@submission.calculate_score))
+        # To enable hints when scoring a submission, uncomment the next line:
+        # send_hints(tubesock, StructuredError.where(submission: @submission))
+        kill_client_socket(tubesock)
+      end
     rescue Runner::Error => e
       tubesock.send_data JSON.dump({cmd: :status, status: :container_depleted})
-      Rails.logger.debug { "Runner error while scoring submission #{@submission.id}: #{e.message}" }
-    ensure
       kill_client_socket(tubesock)
+      Rails.logger.debug { "Runner error while scoring submission #{@submission.id}: #{e.message}" }
     end
   end
 
@@ -196,14 +201,16 @@ class SubmissionsController < ApplicationController
 
   def test
     hijack do |tubesock|
-      return kill_client_socket(tubesock) if @embed_options[:disable_run]
+      tubesock.onopen do |_event|
+        kill_client_socket(tubesock) if @embed_options[:disable_run]
 
-      tubesock.send_data(JSON.dump(@submission.test(@file)))
+        tubesock.send_data(JSON.dump(@submission.test(@file)))
+        kill_client_socket(tubesock)
+      end
     rescue Runner::Error => e
       tubesock.send_data JSON.dump({cmd: :status, status: :container_depleted})
-      Rails.logger.debug { "Runner error while testing submission #{@submission.id}: #{e.message}" }
-    ensure
       kill_client_socket(tubesock)
+      Rails.logger.debug { "Runner error while testing submission #{@submission.id}: #{e.message}" }
     end
   end