A job that outlived its concurrency lock releases the lock of the job that replaced it #827

Summary

When a job outlives its limits_concurrency duration and concurrency maintenance expires its semaphore, the next job with the same key creates a fresh semaphore and runs. When the first job then finishes, its release lands on that new semaphore and unblocks another job while the new holder is still running, well inside its own duration. The extra slot then carries down the backlog: each job that finishes releases the next one early, so a limit of 1 runs two at a time until the key's backlog drains.

How this differs from #735 and #771

Those settled that the job whose lock expired may overlap the job after it. That's the documented trade-off, and this isn't asking to revisit it. This is about the jobs after that one: B and C below never outlive their durations, yet C runs alongside B. The README says the limit "will last for duration for each job enqueued", and B gets less than that. It's the same kind of over-limit admission #761 fixed for pruned workers, by a route exactly-once finalization doesn't cover: A really does own its claim, but the semaphore it releases is no longer the one it acquired.

Steps to reproduce

Two tests. They use no dummy-app models, so they should drop into test/models/solid_queue/. Both fail on 1.7.0:

require "test_helper"

class SolidQueue::ExpiredLockReleaseTest < ActiveSupport::TestCase
  self.use_transactional_tests = false

  class NonOverlappingJob < ActiveJob::Base
    limits_concurrency key: "expired-lock-release", duration: 1.minute

    def perform(name)
    end
  end

  setup do
    @process = SolidQueue::Process.register(kind: "Worker", pid: 42, name: "worker-123")
  end

  test "a job that outlived its lock does not release the lock a later job acquired" do
    a = claim(NonOverlappingJob.perform_later("A"))

    # A runs past its 1-minute duration and concurrency maintenance expires its semaphore
    travel_to 2.minutes.from_now
    SolidQueue::Semaphore.expired.delete_all

    b = claim(NonOverlappingJob.perform_later("B"))
    c = job_for(NonOverlappingJob.perform_later("C"))
    assert c.blocked?

    a.perform

    assert b.job.reload.claimed?
    assert c.reload.blocked?, "C was unblocked while B, 0s into its 1-minute lock, is still running"
  end

  test "a job that outlived its lock does not leave the key over its limit" do
    a = claim(NonOverlappingJob.perform_later("A"))

    travel_to 2.minutes.from_now
    SolidQueue::Semaphore.expired.delete_all

    b = claim(NonOverlappingJob.perform_later("B"))
    c = job_for(NonOverlappingJob.perform_later("C"))
    d = job_for(NonOverlappingJob.perform_later("D"))

    a.perform
    b.perform
    claim_job(c)

    assert d.reload.blocked?, "D was unblocked while C is still running"
  end

  private
    def job_for(active_job)
      SolidQueue::Job.find_by!(active_job_id: active_job.job_id)
    end

    def claim(active_job)
      claim_job(job_for(active_job))
    end

    def claim_job(job)
      SolidQueue::ReadyExecution.claim(job.queue_name, 1, @process.id)
      SolidQueue::ClaimedExecution.find_by!(job_id: job.id)
    end
end

Failure:
SolidQueue::ExpiredLockReleaseTest#test_a_job_that_outlived_its_lock_does_not_release_the_lock_a_later_job_acquired:
C was unblocked while B, 0s into its 1-minute lock, is still running

Failure:
SolidQueue::ExpiredLockReleaseTest#test_a_job_that_outlived_its_lock_does_not_leave_the_key_over_its_limit:
D was unblocked while C is still running

The sequence:

1. A is claimed. Its semaphore expires a minute later.
2. A runs past that, or waits in the ready queue past it behind a backlog. Concurrency maintenance deletes the expired semaphore.
3. B finds no semaphore, creates one with value 0, and is claimed. C and D block on it.
4. A finishes. ClaimedExecution#finalize calls job.unblock_next_blocked_job: Semaphore.signal increments B's semaphore to 1, and release_next_blocked_job promotes C.
5. B finishes, and its release promotes D while C is running. The key stays one over its limit until no blocked job is left to promote.

Expected behavior

A's release does not touch a semaphore created after its own expired, and C stays blocked until B finishes.

Actual behavior

C is unblocked while B is running, and the key runs one over its limit until its backlog drains.

Root cause

Semaphore::Proxy#attempt_increment updates by key alone (where(key: key, value: ...limit)). The semaphore records a count rather than its holders, so it cannot tell a release by a current holder from one by a job whose semaphore was expired and deleted. The value < limit cap stops the count going above the limit, but not a release made on another job's behalf.

Possible fix

Expiring a semaphore deletes its row, and the next acquisition creates a new one, so the row's id already marks a lock generation. If a job recorded the id of the semaphore it acquired, signal could increment where(id: acquired_id) instead of by key, and a release from a job whose row had been expired would match nothing. That holds for to: > 1 as well, since every holder of one row shares its id. It needs somewhere to record the id, so this is a sketch rather than a proposal.

Environment

• solid_queue 1.7.0; semaphore.rb and the finalize and release path are unchanged on main at 73602fe
• Rails 8.1
• Ruby 3.4.7
• Reproduced on SQLite. The sequence is sequential on one connection, with no lock contention involved.
• Found by reading the release path and reproduced in tests; not observed in production