From 291ad54d0c30ee297e504b10d396f3d7267ed249 Mon Sep 17 00:00:00 2001 From: Takumasa Ochi Date: Fri, 2 Nov 2018 23:05:43 +0900 Subject: [PATCH 1/2] Fix broken thread safety by widening critical section Previously, the equality between cache.key and new_key was evaluated outside the critical section of `caches.synchronize`. When these 2 keys are evaluated as different by concurrent processes, caches[cache.key] is deleted more than once and `nil` is assigned to caches[new_key] after 1 thread passed critical section. This patch fix the issue by evaluating equality inside the critical section of `caches.synchronize`. --- lib/sshkit/backends/connection_pool.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sshkit/backends/connection_pool.rb b/lib/sshkit/backends/connection_pool.rb index c7cd4675..37d69eeb 100644 --- a/lib/sshkit/backends/connection_pool.rb +++ b/lib/sshkit/backends/connection_pool.rb @@ -118,9 +118,9 @@ def thread_safe_find_or_create_cache(key) # Update cache key with changed args to prevent cache miss def update_key_if_args_changed(cache, args) new_key = cache_key_for_connection_args(args) - return if cache.same_key?(new_key) caches.synchronize do + return if cache.same_key?(new_key) caches[new_key] = caches.delete(cache.key) cache.key = new_key end From 918cfd86cc89366ed3593279c2a0a803b6e20d99 Mon Sep 17 00:00:00 2001 From: Takumasa Ochi Date: Fri, 18 Jan 2019 21:39:24 +0900 Subject: [PATCH 2/2] Update README.md for #447 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ebb6cf4..dfe84b3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ appear at the top. ## [Unreleased][] * Your contribution here! + * [#447](https://github.com/capistrano/sshkit/pull/447): Fix broken thread safety by widening critical section - [Takumasa Ochi](https://github.com/aeroastro) ## [1.18.0][] (2018-10-21)