[10.x] Fixes ShouldBeUnique behavior for missing models
#50211
Conversation
naquad
changed the title
crynobone
changed the title
ShouldBeUnique behavior for missing models
|
@naquad could you please add a much more thorough explanation for this one? That'll give a much higher chance of it being accepted. |
Updated, please let me know if that's good enough. |
|
Thanks for the PR but the explanation is not clear. What are two simple jobs I can paste into my application that will recreate this issue? |
The original issue contains a PoC along with the reproduction steps: #49890 (comment) (not copy-pasting to avoid the clutter). |
|
@naquad remember Taylor doesn't sees draft PR's. Mark this as ready if you want another review. |
|
Your reproduction steps are too long and unclear. Why are there table create statements in them, etc? |
|
@driesvints could you please help me? I've been trying to make the STR as succint and simple as possible, reducing everything down to a self-contained script exposing the issue, so I'm not sure how to simplify the three steps :( |
|
@naquad best is if you create a laravel app with a separate commit with custom changes to reproduce the issue: laravel new bug-report --github="--public" |
|
@driesvints Like this? |
|
Yes that looks good |
|
@taylorotwell please review the following PoC app: https://github.com/naquad/laravel-bug-report-49890 The |
|
I don't have the bandwidth to review this right now. If you can fix the issue in a much simpler way please submit a simpler solution that doesn't require so many code changes. |
This PR is a follow-up on #49890 and tries to fix the issue by using a memo object that stores the information required to manage the unique lock (uniqueId and the name of the uniqueVia storage if present).
EDIT: A detailed explanation.
The unique job lock mechanism relies on the attributes and methods of the job class. However, these methods are not necessarily deterministic. For example, they might produce different outcomes or errors when called on the same object, especially if dependent objects have changed or no longer exist.
Additionally, if the job incorporates the
Illuminate\Queue\SerializesModelstrait and fails to unserialize due to a missing model, the unserialization process is interrupted by anIlluminate\Database\Eloquent\ModelNotFoundException.Due to these factors, it may become impossible to retrieve the necessary information for managing the unique lock in the queue worker.
This scenario creates a special case where the job's failure to unserialize prevents the worker from releasing the lock, potentially causing a complete deadlock for that particular job class.
To resolve this issue, this pull request introduces a memo object
Illuminate\Queue\UniqueHandler. This object gathers all essential information from the job object beforehand, which can later be used to manage the unique lock w/o relying on the command instance. This approach effectively separates the job lock information from the job object itself and guarantees that both the dispatcher and the worker are working with the same unique lock.