Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix two problems with :concurrent_connections option #4

Merged
merged 2 commits into from Apr 11, 2015

Conversation

sersut
Copy link

@sersut sersut commented Jun 13, 2013

There are two issues right now in net-ssh-multi with :concurrent_connections option:

1-) There is a race condition while fetching the next_session when :concurrent_connections are set. @open_connections is being incremented by the connection thread and sometimes realize_pending_connections!() method can create more than required connection threads before the @open_connections is set by the previously created threads.

2-) When :concurrent_connections is set, server classes are setup with PendingConnection objects that always return true to busy? calls. If a connection fails when :concurrent_connections is set, server ends up returning true to all busy? calls since the session object is not replaced. Due to this, main event loop (process() function) never gets terminated.

1 causes chef not to honour the -C option in knife. 2 causes knife ssh to hang if -C option is specified.

This PR contains fixes these issues both.

@dudemcbacon
Copy link

Is it possible we could get this merged? These bugs are affecting my project as well.

delano added a commit that referenced this pull request Apr 11, 2015
Fix two problems with :concurrent_connections option
@delano delano merged commit 36b55e6 into net-ssh:master Apr 11, 2015
@delano
Copy link
Collaborator

delano commented Apr 11, 2015

Wow, that took almost two years. That's my new record.

Apologies for the delay! Thanks @sersut for the fixes and @dudemcbacon for the reminder. I'll push a new release to Rubygems shortly.

@delano
Copy link
Collaborator

delano commented Apr 11, 2015

@dudemcbacon
Copy link

Much appreciated! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants