get_user_submission susceptible to race condition resulting in invalid DB state
Description
Testing Instructions
Test that the database cannot get in an invalid state
Individual submissions
Create a course with an assignment (default settings are fine) and a user
As the user, in two separate sessions (use a regular browser session and an incognito session) browse to the course but do not click the assignment
Hack
get_user_submission
modifying theif ($create)
block to look like the following:mod/assign/locallib.php
if ($create) { sleep(30); $submission = new stdClass(); $submission->assignment = $this->get_instance()->id; $submission->userid = $userid; $submission->timecreated = time(); $submission->timemodified = $submission->timecreated;
Click the assignment in the normal browser session
While it is sleeping, click the assignment in the incognito window
Wait for both to finish:
The first one will finish without error
The second one will finish and display an error about a unique constraint violation (this is expected)
Verify that there is a record in the
mdl_assign_submission
table corresponding to the user's ID and that the latest column is set to 1
Group submissions
Create a course with a user and a group (ensure the user is in the group)
Create an assignment with group submissions enabled:
Add the assignment activity
Under "Group submission settings" set "Students submit in groups" to "Yes"
As the user, in two separate sessions (use a regular browser session and an incognito session) browse to the course but do not click the assignment
Hack
get_group_submission
modifying theif ($create)
block to look like the following:mod/assign/locallib.php
if ($create) { sleep(30); $submission = new stdClass(); $submission->assignment = $this->get_instance()->id; $submission->userid = $userid; $submission->timecreated = time(); $submission->timemodified = $submission->timecreated;
Click the assignment in the normal browser session
While it is sleeping, click the assignment in the incognito window
Wait for both to finish:
The first one will finish without error
The second one will finish and display an error about a unique constraint violation (this is expected)
Verify that there is a record in the
mdl_assign_submission
table corresponding to the user's ID and that the latest column is set to 1 (double check that the group ID is non-zero to be sure you are looking at the correct record)
Regression testing
Invidual submissions
Create a course with an assignment (default settings are fine) and a user
As the user, browse to the assignment
Verify that there is a record in the
mdl_assign_submission
table corresponding to the user's ID and that the latest column is set to 1
Group submissions
Create a course with a user and a group (ensure the user is in the group)
Create an assignment with group submissions enabled:
Add the assignment activity
Under "Group submission settings" set "Students submit in groups" to "Yes"
As the user, browse to the assignment
Verify that there is a record in the
mdl_assign_submission
table corresponding to the user's ID and that the latest column is set to 1 (double check that the group ID is non-zero to be sure you are looking at the correct record)
Automated test results
Pre-check results
Workaround
Attachments
has a non-specific relationship to
has been marked as being related by
Activity
Thanks for your contributions! This change is now available from the main moodle.git repository and will shortly be available on download.moodle.org.
Closing as fixed!
Hi @Qihui Chan,
you've been added to the developers group in the tracker so you can become the assignee of this and other, future issues. Being in the developers group enables you to request peer-reviews of your issues and other developer superpowers.
Many thanks for your collaboration, ciao
Hi all!
I have tested on integration master and 311 and everything works as expected.
Thanks @Qihui Chan for working on this patch and addressing the raised points. And thanks @Jun for fixing the version numbers.
Integrated to 311, 400 and master!
Over to testing!
I have provided a patch. Please have a look:
400
Diff URL: https://github.com/junpataleta/moodle/commit/MDL-70480-400-fix
Pull:
git checkout MOODLE_400_STABLE && git pull https://github.com/junpataleta/moodle.git MDL-70480-400-fix
311
Diff URL: https://github.com/junpataleta/moodle/commit/MDL-70480-311-fix
Pull:
git checkout MOODLE_311_STABLE && git pull https://github.com/junpataleta/moodle.git MDL-70480-311-fix
Details
Details
Priority
Components
Assignee
Reporter
Peer reviewer
Integrator
Tester
Participants
Pull from Repository
Pull Main Branch
Component Lead Review
Clockify
Start / Stop
Clockify

Probable root cause
After investigating the
get_user_submission
function, it appears it is susceptible to a race condition that leaves the submission table in an invalid state (i.e., every record for a specific student's assignment will have the latest flag set to 0). To reproduce:Create a course with an assignment and a user
As the user, in two separate sessions (use a regular browser session and an incognito session) browse to the course but do not click the assignment
Hack
get_user_submission
modifying theif ($create)
block to look like the following:mod/assign/locallib.php
if ($create) { sleep(30); $submission = new stdClass(); $submission->assignment = $this->get_instance()->id; $submission->userid = $userid; $submission->timecreated = time(); $submission->timemodified = $submission->timecreated;
Click the assignment in the normal browser session
While it is sleeping, click the assignment in the incognito window
Wait for both to finish
The first one will complete; at this point a valid record is in the DB. But when the second one finishes sleeping and gets to this block:
mod/assign/locallib.php
if ($submission->latest) { // This is the case when we need to set latest to 0 for all the other attempts. $DB->set_field('assign_submission', 'latest', 0, $params); } $sid = $DB->insert_record('assign_submission', $submission); return $DB->get_record('assign_submission', array('id' => $sid));
The query to set everything to zero (i.e.,
$DB->set_field('assign_submission', 'latest', 0, $params);
but the query to add a new (valid) submission fails (i.e.,$sid = $DB->insert_record('assign_submission', $submission);
) due to a unique constraint.Now the DB has one record with latest set to 0 and we observe that the view in the assignment table does not match the grader (which could be a bug in its own right).
Original description
When the number of users started rising sharply, we implemented a master/slave MariaDB 10.3.17 configuration for our Moodle 3.9.2 installation to alleviate some of the stress on the database, as we have also started receiving DDOS attacks.
Some of our users have started to notice some assignment submissions missing from the grading form in the assignment module. The form lists the users entry as not submitted, but the "Edit submission" or "Grade" option shows all the submitted files correctly.
After some database querying, we have found that the column "latest" in the table "mdl_assign_submission" is set to 0 on the users only submission entry for that assignment (status="submitted"), which was the reason for it not showing up on the grading forum.
As we have also had some problems with assignment notification mails for some users - the column "mailed" in the table "mdl_assign_user_flags" could not be set to 1 during the cron task, possibly due to the database sync time, which resulted in some users getting the same notification on every time the cron task ran.
We have since rolled back to a single database setup. We manually set the "mailed" flag to 1 for the users, affected by that previous problem, to stop receiving spam every minute.
We found all of the missing assignment submissions with the sql query:
SELECT s.id,s.assignment,s.userid,s.status,s.latest FROM mdl_assign_submission AS s JOIN mdl_course_modules AS m ON s.assignment=m.instance WHERE s.status="submitted" AND s.latest=0 AND (SELECT count(*) FROM mdl_assign_submission WHERE assignment = s.assignment AND userid = s.userid ) = 1 AND m.module=1;
and fixed them manually by running
UPDATE mdl_assign_submission AS s SET s.latest=1 WHERE s.status="submitted" AND s.latest=0 AND (SELECT count(*) FROM mdl_assign_submission WHERE assignment = s.assignment AND userid = s.userid ) = 1;
We have confirmed that the problems started when we switched to master/slave DB configuration and that no newly created submissions after the rollback to single database instance have experienced the same problem, so it would make sense to blame the database replica synchronization time interval. The problem does, however, persist on submissions that have been created during that time period and have since been updated.
Unfortunately, we have not been able to reproduce the problem, only its symptoms (setting the column "latest" to 0).
Any ideas as to what may have happened? Any help would be greatly appreciated.