Skip to content

[19.0] [FIX] queue_job: deadlock when the on fail hook writes the job's records - #1001

Open
ivantodorovich wants to merge 1 commit into
OCA:19.0from
camptocamp:19.0-fix-queue_job-on_fail-deadlock
Open

ivantodorovich wants to merge 1 commit into
OCA:19.0from
camptocamp:19.0-fix-queue_job-on_fail-deadlock

Conversation

@ivantodorovich

Copy link
Copy Markdown
Contributor

When a job fails, _runjob records the failure and calls the on_fail hook from a temporary cursor, while the job's own transaction is still open.

If the hook writes on a record the job wrote, it waits for the job's lock, which is only released after the hook. The job stays started forever -> deadlock


This PR runs the job inside a savepoint: when the job fails, rolling back the savepoint releases the locks it took. The job lock on queue_job_lock was taken before the savepoint, so it stays held.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @guewen, @sbidoul,
some modules you are maintaining are being modified, check this out!

@simahawk simahawk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LG

Comment thread queue_job/controllers/main.py
Comment thread queue_job/controllers/main.py Outdated
The failure is handled from a temporary cursor while the failed job's
transaction still holds its locks: an on fail hook writing on the same
records waited forever. Perform the job in a savepoint so that its
locks are released when it fails.
@ivantodorovich
ivantodorovich force-pushed the 19.0-fix-queue_job-on_fail-deadlock branch from d33125d to c51fe59 Compare September 30, 2026 13:57

@grindtildeath grindtildeath left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice 👌

@UsmanGhias UsmanGhias left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Clean patch for OCA/queue.

Observations:

  • SQL query correctly uses parameterized %s arguments for cr.execute, maintaining clean injection safety.

Ready for testing on standard environments.

Regards,
Usman
https://usmanghias.co.uk

@UsmanGhias UsmanGhias left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work on this PR! Fixing this deadlock scenario is a massive win for high-concurrency Odoo environments running heavy background queues.

Technically, wrapping the execution block inside env.cr.savepoint() is a clean way to drop row-level locks on failure while safely preserving the session-level advisory locks via queue_job_lock. I also appreciate how you structured the regression test using a separate cursor and a lock timeout to properly simulate the multi-connection concurrency issue without flakiness.

Just a small heads-up on the truncated test code at the end of the patch, but the logic itself is rock-solid. Excited to see this land in version 19!

Regards,
Usman
https://usmanghias.co.uk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants