Skip to content

Commit

Permalink
Document concurrent memoization of zero-arg methods (#230)
Browse files Browse the repository at this point in the history
Document, via both README and specs, that when two threads concurrently
call a zero-arg method which is in the unmemoized state, they can return
two different results.

For example, this could occur if we are memoizing:
  * database query result
  * filesystem read
  * current time
  * a random number

This PR introduces a test which probabilistically reproduces the
issue, as well as documentation that this is the expected behavior.

Disables a test on TruffleRuby in CI, as it takes multiple seconds
to observe the different return values there.

Closes #184
  • Loading branch information
ms-ati authored Dec 3, 2021
1 parent f2cdecf commit a97cd40
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 11 deletions.
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,24 @@ $ bundle exec ruby benchmarks.rb
If your results differ from what's posted here,
[let us know](https://github.com/panorama-ed/memo_wise/issues/new)!

## Thread Safety

MemoWise makes the following **thread safety** guarantees on all supported Ruby
versions:

1. **Before** a value has been memoized

* Contended calls from multiple threads...
* May each call the original method
* May return different valid results (when the method is nondeterministic,
like `rand`)
* Will memoize exactly one valid return value

2. **After** a value has been memoized

* Contended calls from multiple threads...
* Always return the same memoized value

## Documentation

### Documentation is Automatically Generated
Expand Down
63 changes: 52 additions & 11 deletions spec/thread_safety_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
RSpec.describe "thread safety" do # rubocop:disable RSpec/DescribeClass
context "when two threads accessing unmemoized zero-args method" do
let(:thread_return_values) do
any_nils = ->(a) { a.any?(&:nil?) }

check_repeatedly(condition_proc: any_nils) do
instance = class_with_memo.new
threads = Array.new(2) { Thread.new { instance.first_thread_id } }
check_repeatedly(condition_proc: condition_to_check) do
@instance = class_with_memo.new
threads = Array.new(2) { Thread.new { @instance.current_thread_id } } # rubocop:disable RSpec/InstanceVariable
threads.map(&:value)
end
end
Expand All @@ -16,20 +14,63 @@
Class.new do
prepend MemoWise

def first_thread_id
Thread.pass # trigger a race condition even on MRI
Thread.current.object_id
def current_thread_id
Thread.pass # trigger a race condition even on MRI
Thread.current.object_id # return different values in each thread
end
memo_wise :first_thread_id
memo_wise :current_thread_id
end
end

# Tip of the hat to @jeremyevans for finding and fixing a race condition
# which caused accidental `nil` values to be returned under contended calls
# to unmemoized zero-args methods.
# * See https://github.com/panorama-ed/memo_wise/pull/224
it "does not return accidental nil value to either thread" do
expect(thread_return_values).not_to include(nil)
context "when checking condition: are nil values accidentally returned?" do
let(:condition_to_check) do
->(values) { values.any?(&:nil?) }
end

it "does not return accidental nil value to either thread" do
expect(thread_return_values).not_to include(nil)
end
end

# When memoizing non-deterministic methods, MemoWise does not impose any
# additional guarantees other than:
# * Return values are one of the valid returns from the original method
# * Return values may differ between threads under contended calls
# * One of those return values will be memoized and returned thereafter
context "when checking condition: are different values returned?" do
let(:condition_to_check) do
->(values) { values.uniq.size > 1 }
end

let(:after_memoization_thread_return_values) do
thread_return_values # Ensure threads have already executed

check_repeatedly(condition_proc: condition_to_check) do
threads = Array.new(2) { Thread.new { @instance.current_thread_id } } # rubocop:disable RSpec/InstanceVariable
threads.map(&:value)
end
end

# NOTE: Disabled on TruffleRuby in CI, because it takes multiple seconds
# to observe the different values returned to multiple threads, and errors
# out in some cases when that does happen.
unless RUBY_ENGINE == "truffleruby" && ENV["CI"] == "true"
it "returns different values to each thread, and memoizes one of them" do
# Before memoization: expect to observe threads return different values
expect(thread_return_values.uniq.size).to be > 1

# After memoization: expect to observe only a single value...
expect(after_memoization_thread_return_values.uniq.size).to be 1

# ...and that single value is one the values returned originally
expect(thread_return_values).
to include(after_memoization_thread_return_values.first)
end
end
end
end
end

0 comments on commit a97cd40

Please sign in to comment.