rsim / oracle-enhanced

Oracle enhaced adapter for ActiveRecord
MIT License
545 stars 308 forks source link

Handle multiple threads in rails system tests #2287

Open throwern opened 2 years ago

throwern commented 2 years ago

I've been working on upgrading rspec tests in a Rails 6.0.4 application using activerecord-oracle_enhanced-adapter v6.0.6. This update demonstrates a bug we encountered and a potential fix. The bug RuntimeError: executing in another thread occurs for us after the following:

The error seems to be a race condition and is difficult to reproduce consistently. After tracing through the code, I think it is related to a feature added in Rails 5.1 to support transactions in system tests: https://github.com/rails/rails/pull/28083

The update made all threads share the same database connection in tests. It uses Monitor and synchronize to avoid simultaneous requests from different threads. For example, anything using the log method is wrapped with a call to synchronize: https://github.com/rails/rails/blob/v7.0.2.3/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb#L765

It seems that some methods in the oracle adapter are not synchronized. For our project, the errors traced back to the OracleEnhanced::Connection#describe method in connection.rb. I was able to fix the error with calls to @lock.synchronize around any method using describe. Those changes are included in this commit.

I'm not deeply familiar with this adapter or ruby threads so this is more of a quick patch and proof-of-concept. I included a test case that usually reproduces the error without the fix. Hopefully someone more familiar with these codebases could take a look.

Thanks!