Skip to content

Commit

Permalink
Fix fragile specs (#1041)
Browse files Browse the repository at this point in the history
* Use monotonic time if possible to guard against clock skew
* Relax time comparisons to make test failures under load less likely
* Fix memory corruption segfaults on macOS due to mixing Threads and Timeout

Without the timeout thread safety fix, the specs always crash in the
"threaded queries should be supported" spec for various reasons on
macOS 10.14.4 with ruby 2.6.1 / MySQL 8.0.15.

Some observed errors:

    malloc: *** error for object 0x7fcbba90ef80: pointer being freed was not allocated
    malloc: *** set a breakpoint in malloc_error_break to debug

    lib/mysql2/client.rb:131:in `_query': Bad file descriptor (Errno::EBADF)

Now specs run fine, even during Prime95 load test on i9-8950HK (6c/12t).
  • Loading branch information
felixbuenemann authored and sodabrew committed Apr 27, 2019
1 parent d7c91ff commit c8c346d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 9 deletions.
21 changes: 12 additions & 9 deletions spec/mysql2/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -607,29 +607,29 @@ def run_gc
# XXX this test is not deterministic (because Unix signal handling is not)
# and may fail on a loaded system
it "should run signal handlers while waiting for a response" do
kill_time = 0.1
query_time = 2 * kill_time
kill_time = 0.25
query_time = 4 * kill_time

mark = {}

begin
trap(:USR1) { mark.store(:USR1, Time.now) }
trap(:USR1) { mark.store(:USR1, clock_time) }
pid = fork do
sleep kill_time # wait for client query to start
Process.kill(:USR1, Process.ppid)
sleep # wait for explicit kill to prevent GC disconnect
end
mark.store(:QUERY_START, Time.now)
mark.store(:QUERY_START, clock_time)
@client.query("SELECT SLEEP(#{query_time})")
mark.store(:QUERY_END, Time.now)
mark.store(:QUERY_END, clock_time)
ensure
Process.kill(:TERM, pid)
Process.waitpid2(pid)
trap(:USR1, 'DEFAULT')
end

# the query ran uninterrupted
expect(mark.fetch(:QUERY_END) - mark.fetch(:QUERY_START)).to be_within(0.02).of(query_time)
expect(mark.fetch(:QUERY_END) - mark.fetch(:QUERY_START)).to be_within(0.1).of(query_time)
# signals fired while the query was running
expect(mark.fetch(:USR1)).to be_between(mark.fetch(:QUERY_START), mark.fetch(:QUERY_END))
end
Expand Down Expand Up @@ -694,6 +694,7 @@ def run_gc
sleep_time = 0.5

# Note that each thread opens its own database connection
start = clock_time
threads = Array.new(5) do
Thread.new do
new_client do |client|
Expand All @@ -702,10 +703,12 @@ def run_gc
Thread.current.object_id
end
end
values = threads.map(&:value)
stop = clock_time

# This timeout demonstrates that the threads are sleeping concurrently:
# In the serial case, the timeout would fire and the test would fail
values = Timeout.timeout(sleep_time * 1.1) { threads.map(&:value) }
# This check demonstrates that the threads are sleeping concurrently:
# In the serial case, the difference would be a multiple of sleep time
expect(stop - start).to be_within(0.1).of(sleep_time)

expect(values).to match_array(threads.map(&:object_id))
end
Expand Down
11 changes: 11 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ def num_classes
# rubocop:enable Lint/UnifiedInteger
end

# Use monotonic time if possible (ruby >= 2.1.0)
if defined?(Process::CLOCK_MONOTONIC)
def clock_time
Process.clock_gettime Process::CLOCK_MONOTONIC
end
else
def clock_time
Time.now.to_f
end
end

config.before :each do
@client = new_client
end
Expand Down

0 comments on commit c8c346d

Please sign in to comment.