Skip to content

Commit

Permalink
Permit concurrent loads to avoid a deadlock
Browse files Browse the repository at this point in the history
https://bugzilla.redhat.com/show_bug.cgi?id=1596703
https://bugzilla.redhat.com/show_bug.cgi?id=1595832

If two web service worker threads both receive a request and have not yet
initialized time_attributes, it's possible for the threads to deadlock as
described below.

Thread 1:
constantizes CloudObjectStoreContainer, which tries to include_concern
"Operations" (note: the constant is not yet loaded)... and waits for a
lock held by thread 2

Thread 2:
constantizes CloudObjectStoreContainer, acquires a lock and sits and
waits on something else

It's unclear why thread 2 is holding a lock and will not relinquish it.

For now, we can protect this constantize by wrapping
each constantize in this method in a permit_concurrent_loads block.

Note, several other classes successfully load concurrently, but
CloudObjectStoreContainer consistently hits this.  It is the first class
that tries to constantize that also does include_concern so it's
possible this combination will be responsible for the eventual fix.

To test this:

1) Add rails lock debugging middleware to config/application.rb:
   `config.middleware.insert_before Rack::Sendfile, ActionDispatch::DebugLocks`

2) Copy this script to /var/www/miq/vmdb:

Thread.new do
  `curl -k -L https://admin:smartvm@localhost/api/notifications?expand=resources&attributes=details&sort_by=id&sort_order=desc&limit=100`
end

Thread.new do
  `curl -k -L https://admin:smartvm@localhost/api/notifications?expand=resources&attributes=details&sort_by=id&sort_order=desc&limit=100`
end

3) `systemctl start evmserverd` (or restart if you added the middleware in
step 1)

4) run the script in step 2.  `ruby path/to/file`.

5) Hit the locks endpoint added in step 1): `curl -L http://admin:smartvm@localhost:4000/rails/locks`

6) If the locks endpoint shows locks, run it again as it could be a
temporary lock.  With the fixed code, the curl command should have no
output.  With the broken code, it outputs the threads, their backtraces
and who's blocking whom.
  • Loading branch information
jrafanie committed Jul 10, 2018
1 parent 74851f1 commit abd070a
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion lib/api/environment.rb
Expand Up @@ -17,7 +17,12 @@ def self.encrypted_attributes
def self.time_attributes
@time_attributes ||= ApiConfig.collections.each.with_object(Set.new(%w(expires_on))) do |(_, cspec), result|
next if cspec[:klass].blank?
klass = cspec[:klass].constantize
klass = nil

# Temporary measure to avoid thread race condition which could lead to a deadlock
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
klass = cspec[:klass].constantize
end
klass.columns_hash.each do |name, typeobj|
result << name if %w(date datetime).include?(typeobj.type.to_s)
end
Expand Down

0 comments on commit abd070a

Please sign in to comment.