Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-70480

get_user_submission susceptible to race condition resulting in invalid DB state

    XMLWordPrintable

Details

    • MOODLE_311_STABLE, MOODLE_400_STABLE
    • MOODLE_311_STABLE, MOODLE_400_STABLE
    • MDL-70480-master
    • Hide

      Test that the database cannot get in an invalid state

      Individual submissions

      1. Create a course with an assignment (default settings are fine) and a user
      2. 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
      3. Hack get_user_submission modifying the if ($create) block to look like the following:

        mod/assign/locallib.php

        3924
        if ($create) {
        3925
            sleep(30);
        3926
            $submission = new stdClass();
        3927
            $submission->assignment   = $this->get_instance()->id;
        3928
            $submission->userid       = $userid;
        3929
            $submission->timecreated = time();
        3930
            $submission->timemodified = $submission->timecreated;
        

      4. Click the assignment in the normal browser session
      5. While it is sleeping, click the assignment in the incognito window
      6. 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)
      7. 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

      1. Create a course with a user and a group (ensure the user is in the group)
      2. Create an assignment with group submissions enabled:
        1. Add the assignment activity
        2. Under "Group submission settings" set "Students submit in groups" to "Yes"
      3. 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
      4. Hack get_group_submission modifying the if ($create) block to look like the following:

        mod/assign/locallib.php

        3927
        if ($create) {
        3928
            sleep(30);
        3929
            $submission = new stdClass();
        3930
            $submission->assignment   = $this->get_instance()->id;
        3931
            $submission->userid       = $userid;
        3932
            $submission->timecreated = time();
        3933
            $submission->timemodified = $submission->timecreated;
        

      5. Click the assignment in the normal browser session
      6. While it is sleeping, click the assignment in the incognito window
      7. 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)
      8. 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

      1. Create a course with an assignment (default settings are fine) and a user
      2. As the user, browse to the assignment
      3. 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

      1. Create a course with a user and a group (ensure the user is in the group)
      2. Create an assignment with group submissions enabled:
        1. Add the assignment activity
      3. Under "Group submission settings" set "Students submit in groups" to "Yes"
      4. As the user, browse to the assignment
      5. 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)
      Show
      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 the if ($create) block to look like the following: mod/assign/locallib.php 3924 if ( $create ) { 3925 sleep(30); 3926 $submission = new stdClass(); 3927 $submission ->assignment = $this ->get_instance()->id; 3928 $submission ->userid = $userid ; 3929 $submission ->timecreated = time(); 3930 $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 the if ($create) block to look like the following: mod/assign/locallib.php 3927 if ( $create ) { 3928 sleep(30); 3929 $submission = new stdClass(); 3930 $submission ->assignment = $this ->get_instance()->id; 3931 $submission ->userid = $userid ; 3932 $submission ->timecreated = time(); 3933 $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)

    Description

      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:

      1. Create a course with an assignment and a user
      2. 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
      3. Hack get_user_submission modifying the if ($create) block to look like the following:

        mod/assign/locallib.php

        3924
        if ($create) {
        3925
            sleep(30);
        3926
            $submission = new stdClass();
        3927
            $submission->assignment   = $this->get_instance()->id;
        3928
            $submission->userid       = $userid;
        3929
            $submission->timecreated = time();
        3930
            $submission->timemodified = $submission->timecreated;
        

      4. Click the assignment in the normal browser session
      5. While it is sleeping, click the assignment in the incognito window
      6. 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

      3953
      if ($submission->latest) {
      3954
          // This is the case when we need to set latest to 0 for all the other attempts.
      3955
          $DB->set_field('assign_submission', 'latest', 0, $params);
      3956
      }
      3957
      $sid = $DB->insert_record('assign_submission', $submission);
      3958
      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.

      Attachments

        Issue Links

          Activity

            People

              qihuichan Qihui Chan
              tjazbec Timotej Jazbec
              cameron1729 cameron1729
              Victor Déniz Falcón Victor Déniz Falcón
              Mihail Geshoski Mihail Geshoski
              Votes:
              4 Vote for this issue
              Watchers:
              18 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 5 hours, 47 minutes
                  5h 47m

                  Clockify

                    Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.