Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Job signature system is not thread safe #317

Open
1 task
mfendeksilverstripe opened this issue Aug 23, 2020 · 1 comment · May be fixed by #342
Open
1 task

Job signature system is not thread safe #317

mfendeksilverstripe opened this issue Aug 23, 2020 · 1 comment · May be fixed by #342
Labels
affects/v4 complexity/low impact/low type/bug

Comments

@mfendeksilverstripe
Copy link
Contributor

mfendeksilverstripe commented Aug 23, 2020 •
edited by brynwhyman
Loading

Job signature system is not thread safe

Running multiple instances of dev tasks which create the same jobs may result in multiple copies of the same job instance with the same signature.

Running multiple dev tasks at the same time is not intentional but this may happen in some situations. For example:

  • running a dev task with long execution time via browser may result in the browser thinking that the page is unresponsive and firing a redundant request
  • running a dev task with long execution time as a job may result in the queue runner thinking the task job is stale and resuming it

Suggested solution

Inside QueuedJobService::queueJob() wrap the job creation inside transaction (use withTransaction) and change the check for existing job to use the SELECT ... FOR UPDATE approach (similar to job locking).

PR
@kinglozzer
Copy link
Collaborator

I’ve noticed something that may or may not be related to this issue - errors in jobs are sometimes logged against the wrong job descriptor 😕

danaenz added a commit to silverstripe-terraformers/silverstripe-queuedjobs that referenced this issue Mar 26, 2021
@danaenz
[BUGFIX symbiote#317] Make queueJob threadsafe
c941a73 
- Separate functionality into protected methods for readability
- Wrap queueJob read/write functionality in withTransaction()
@danaenz danaenz linked a pull request Mar 26, 2021 that will close this issue
Open
@brynwhyman brynwhyman removed their assignment Jun 21, 2021
@emteknetnz emteknetnz assigned emteknetnz and unassigned emteknetnz Jun 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
affects/v4 complexity/low impact/low type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[BUGFIX #317] Make queueJob threadsafe silverstripe-terraformers/silverstripe-queuedjobs
5 participants
@kinglozzer @emteknetnz @dnsl48 @brynwhyman @mfendeksilverstripe