Skip to content
Snippets Groups Projects
Unverified Commit 0b0c4e51 authored by Lin Jen-Shin's avatar Lin Jen-Shin Committed by Stan Hu
Browse files

Always shutdown webrick even if thread is not alive

This should properly shutdown webrick so that we don't need to
forcefully kill the thread to shutdown webrick in the tests.

Without properly shutting down webrick, we leak threads and other
resources to each test cases, eventually causing everything to slow down
and timing out the CI jobs.

This is discovered in
https://gitlab.com/gitlab-org/gitlab/-/issues/462928#note_1916591506

A quick fix was proposed in the above thread and merged in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/153887

This should properly shutdown webrick instead.

* It should be safe to shutdown webrick regardless
* In a test, we use a thread to wrap around `WEBrick::HTTPServer`,
  turning the blocking server into a non-blocking server. This means
  the daemon thread will stop immediately because everything is
  wrapped inside another thread. If we only shutdown webrick when
  the thread is alive, we're not shutting it down in this test case
  either, leaking resources.

This preserves similar performance comparing to killing the thread.

This partially reverts
https://gitlab.com/gitlab-org/gitlab/-/commit/f5b1317af8aad2d8b7d098c0e4dada25ccfd924b
parent 6b27b92c
No related branches found
No related tags found
No related merge requests found
Loading
Loading
@@ -68,11 +68,8 @@ def stop_working
if server
# we close sockets if thread is not longer running
# this happens, when the process forks
if thread.alive?
server.shutdown
else
server.listeners.each(&:close)
end
server.listeners.each(&:close) unless thread.alive?
server.shutdown
end
 
@server = nil
Loading
Loading
Loading
Loading
@@ -216,7 +216,7 @@ def call(env)
# in separate thread
allow_any_instance_of(::WEBrick::HTTPServer)
.to receive(:start).and_wrap_original do |m, *args|
Thread.new do
@server_thread = Thread.new do # rubocop:disable RSpec/InstanceVariable -- let does not work for this case
m.call(*args)
rescue IOError
# is raised as we close listeners
Loading
Loading
@@ -224,8 +224,17 @@ def call(env)
end
end
 
attr_reader :server_thread
after do
exporter.stop
next unless server_thread
server_thread.join(0.05)
raise '`exporter.stop` should terminate `server_thread`' if server_thread.alive?
ensure
server_thread.kill.join if server_thread
end
 
with_them do
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment