Skip to content

Conversation

@seuros
Copy link
Member

@seuros seuros commented Jul 5, 2025

fixes #76 - Race condition in advisory_lock_exists?

… do what it's paid for

fixes #76 - Race condition in advisory_lock_exists?
@seuros seuros requested a review from Copilot July 5, 2025 01:26
@seuros seuros changed the title feat:PostgreSQL, try non-blocking query first to avoid race conditions feat: PostgreSQL, try non-blocking query first to avoid race conditions Jul 5, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a non-blocking advisory lock existence check for PostgreSQL to avoid race conditions and updates the fallback to the original implementation. It also introduces multi-threaded tests covering both the new and fallback paths, plus minor cleanups in MySQL and the gemspec.

  • Add advisory_lock_exists_for? in the PostgreSQL adapter for direct pg_locks queries
  • Integrate the new method in advisory_lock_exists?, falling back when needed
  • Introduce multi-threaded tests and a fallback exception test for PostgreSQL
  • Remove unused helper in MySQL adapter and tweak gemspec formatting

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
with_advisory_lock.gemspec Adjusted heredoc formatting to preserve trailing spaces
test/with_advisory_lock/postgresql_race_condition_test.rb Added tests for race conditions, detection, and fallback behavior
lib/with_advisory_lock/postgresql_advisory.rb Implemented advisory_lock_exists_for? and simplified error guard
lib/with_advisory_lock/mysql_advisory.rb Removed unused unique_column_name method
lib/with_advisory_lock/core_advisory.rb Updated advisory_lock_exists? to prefer non-blocking check
Comments suppressed due to low confidence (1)

test/with_advisory_lock/postgresql_race_condition_test.rb:113

  • The fallback test does not simulate a pg_locks access failure, so it never exercises the rescue path. Consider stubbing conn.advisory_lock_exists_for? to raise ActiveRecord::StatementInvalid and asserting that advisory_lock_exists? still returns a Boolean.
      assert_nothing_raised do

@seuros seuros merged commit f7f8dbc into master Jul 5, 2025
6 checks passed
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.

Race condition in advisory_lock_exists?

2 participants