From 57cdd45b4dedb958477234f1a1e41f6b1844aff2 Mon Sep 17 00:00:00 2001 From: Orien Madgwick <497874+orien@users.noreply.github.com> Date: Sun, 28 Dec 2025 11:21:13 +0700 Subject: [PATCH] fix(locking): Replace object_id-based locks with thread-local storage The previous implementation stored locks in a class variable indexed by Thread.current.object_id, which had three critical defects: 1. Object ID reuse: Ruby can reuse object IDs after garbage collection, allowing new threads to inadvertently access stale locks from dead threads 2. Race conditions: The shared @@locks class variable was not thread-safe, creating potential race conditions 3. Memory leaks: Dead thread entries were not automatically cleaned up This fix replaces the class variable with Ruby's built-in thread-local storage (Thread.current[:double_entry_locks]), which: - Ties data directly to the thread object, not its object ID - Provides isolated storage per thread - Automatically cleans up when threads terminate --- CHANGELOG.md | 7 +++++++ lib/double_entry/locking.rb | 8 +++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29d88bfd..3d8ffce3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Fixed + +- Fix critical thread-safety issues in locking mechanism by replacing object_id-based lock storage with proper thread-local storage. This resolves object ID reuse vulnerabilities, race conditions, and memory leaks ([#226]). + +### Changed + - Run the test suite against Rails 8.1, 8.0, 7.2, and Ruby 4.0, 3.4, 3.3, 3.2 ([#225]). [Unreleased]: https://github.com/envato/double_entry/compare/v2.0.1...HEAD [#225]: https://github.com/envato/double_entry/pull/225 +[#226]: https://github.com/envato/double_entry/pull/226 ## [2.0.1] - 2023-11-01 diff --git a/lib/double_entry/locking.rb b/lib/double_entry/locking.rb index 4d6b3f22..00fc250b 100644 --- a/lib/double_entry/locking.rb +++ b/lib/double_entry/locking.rb @@ -57,8 +57,6 @@ def self.balance_for_locked_account(account) end class Lock - @@locks = {} - def initialize(accounts) # Make sure we always lock in the same order, to avoid deadlocks. @accounts = accounts.flatten.sort @@ -97,15 +95,15 @@ def balance_for(account) private def locks - @@locks[Thread.current.object_id] + Thread.current[:double_entry_locks] end def locks=(locks) - @@locks[Thread.current.object_id] = locks + Thread.current[:double_entry_locks] = locks end def remove_locks - @@locks.delete(Thread.current.object_id) + Thread.current[:double_entry_locks] = nil end # Return true if there's a lock on the given account.