Skip to content

Loading…

Use thread-local variables to prevent recursive #connect invocation #146

Merged
merged 1 commit into from

2 participants

@cheald

Instead bail out immediately in a recursive connect scenario. Resolves mutex deadlocks and prevents potential infinite recursion in weird networking circumstances.

Passes normal/RS/sharded_cluster suites under 1.8.7-p371, 1.9.3-p327, and JRuby 1.7.1. Additionally, I ran those suites both against mongod 2.2.2 and mongod 2.3.1, all passing.

@cheald cheald Use thread-local variables to prevent recursive #connect invocations,…
… and instead bail out immediately in a recursive connect scenario. Resolves mutex deadlocks and prevents potential infinite recursion in weird networking circumstances.
ebfdb8f
@TylerBrock

Dope. Thanks Chris. This has been a huge help.

@TylerBrock TylerBrock merged commit c95d07d into mongodb:RUBY-522
@TylerBrock

In the future please make sure the pull requests are against the master branch, thanks!

@cheald

It seems like Jenkins is still upset with these changes against unstable-release; any idea if that's my break? I can't replicate those issues against 2.3.1 in my local environment.

@TylerBrock
@TylerBrock

Apparently these are now test only commands.

see: https://github.com/mongodb/mongo/blob/master/jstests/slowNightly/testing_only_commands.js

We will have to enable test mode in Jenkins I guess.

@cheald

Okay, cool.

Can I recommend renaming that profile to unstable-nightly? I tested against 2.3.1 as it is in fact called the "unstable release" :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 5, 2013
  1. @cheald

    Use thread-local variables to prevent recursive #connect invocations,…

    cheald committed
    … and instead bail out immediately in a recursive connect scenario. Resolves mutex deadlocks and prevents potential infinite recursion in weird networking circumstances.
This page is out of date. Refresh to see the latest.
Showing with 34 additions and 14 deletions.
  1. +17 −8 lib/mongo/mongo_replica_set_client.rb
  2. +17 −6 lib/mongo/mongo_sharded_client.rb
View
25 lib/mongo/mongo_replica_set_client.rb
@@ -182,17 +182,26 @@ def inspect
# Initiate a connection to the replica set.
def connect
log(:info, "Connecting...")
+
+ # Prevent recursive connection attempts from the same thread.
+ # This is done rather than using a Monitor to prevent potentially recursing
+ # infinitely while attempting to connect and continually failing. Instead, fail fast.
+ raise ConnectionFailure, "Failed to get node data." if thread_local[:locks][:connecting] == true
+
@connect_mutex.synchronize do
return if @connected
-
- if @manager
- @manager.refresh! @seeds
- else
- @manager = PoolManager.new(self, @seeds)
- thread_local[:managers][self] = @manager
- @manager.connect
+ begin
+ thread_local[:locks][:connecting] = true
+ if @manager
+ @manager.refresh! @seeds
+ else
+ @manager = PoolManager.new(self, @seeds)
+ thread_local[:managers][self] = @manager
+ @manager.connect
+ end
+ ensure
+ thread_local[:locks][:connecting] = false
end
-
@refresh_version += 1
if @manager.pools.empty?
View
23 lib/mongo/mongo_sharded_client.rb
@@ -91,13 +91,24 @@ def inspect
def connect(force = !@connected)
return unless force
log(:info, "Connecting...")
+
+ # Prevent recursive connection attempts from the same thread.
+ # This is done rather than using a Monitor to prevent potentially recursing
+ # infinitely while attempting to connect and continually failing. Instead, fail fast.
+ raise ConnectionFailure, "Failed to get node data." if thread_local[:locks][:connecting]
+
@connect_mutex.synchronize do
- if @manager
- @manager.refresh! @seeds
- else
- @manager = ShardingPoolManager.new(self, @seeds)
- thread_local[:managers][self] = @manager
- @manager.connect
+ begin
+ thread_local[:locks][:connecting] = true
+ if @manager
+ @manager.refresh! @seeds
+ else
+ @manager = ShardingPoolManager.new(self, @seeds)
+ thread_local[:managers][self] = @manager
+ @manager.connect
+ end
+ ensure
+ thread_local[:locks][:connecting] = false
end
@refresh_version += 1
Something went wrong with that request. Please try again.