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

Duplicate forum subscriptions due to race conditions

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.2.4, 3.3.1, 3.4
    • Fix Version/s: 3.2.6, 3.3.3
    • Component/s: Forum
    • Testing Instructions:
      Hide

      Since the time window, between the execution of the two line, responsible for the race condition, is pretty small. You have to be pretty fast and pretty lucky to actually reproduce this issue (we have only 14 occurrences in 190111 forum subscriptions).

      Thus, I suggest the following procedure to first reproduce this issue and afterwards test the patch.

      First reproduce the issue without the patch:

      1. Create a new forum and subscribe three users to it. (Lets say your user ids are 2, 3 and 4 and your forum id is 5)
      2. Go to the database and execute this select statement (this statement is executed more often later on):

        select * from mdl_forum_subscriptions where forum=5;
        

      3. Result: 3 entry are shown for your three subscriptions.
      4. Go to the database and execute the following statement twice:

        insert into forum_subscriptions (userid, forum) values (2,5);

      5. And the following once:

      insert into mdl_forum_subscriptions (userid, forum) values (3,5);

      # Now the above listed select query should return 6 entries, including one duplicate and one triplet.

      1. Load the list of forum subscriptions, where the mentioned exception is shown.

      Now install the patch (Without resetting the DB state):

      1. The above listed select query should return again 3 entries for both subscriptions, but no longer any duplicate.
      2. Load the list of forum subscriptions. No exception should be thrown!
      3. Go to the database and execute:

        insert into mdl_forum_subscriptions (userid, forum) values (2,5);
        

      1. You should get an error like:
      2. ERROR:  duplicate key value violates unique constraint "mdl_forusubs_usefor_uix"
        DETAIL:  Key (userid, forum)=(2, 5) already exists.
        

      Show
      Since the time window, between the execution of the two line, responsible for the race condition, is pretty small. You have to be pretty fast and pretty lucky to actually reproduce this issue (we have only 14 occurrences in 190111 forum subscriptions). Thus, I suggest the following procedure to first reproduce this issue and afterwards test the patch. First reproduce the issue without the patch: Create a new forum and subscribe three users to it. (Lets say your user ids are 2, 3 and 4 and your forum id is 5 ) Go to the database and execute this select statement (this statement is executed more often later on): select * from mdl_forum_subscriptions where forum= 5 ; Result: 3 entry are shown for your three subscriptions. Go to the database and execute the following statement twice: insert into forum_subscriptions (userid, forum) values ( 2 , 5 ); And the following once: insert into mdl_forum_subscriptions (userid, forum) values ( 3 , 5 ); # Now the above listed select query should return 6 entries, including one duplicate and one triplet. Load the list of forum subscriptions, where the mentioned exception is shown. Now install the patch ( Without resetting the DB state ): The above listed select query should return again 3 entries for both subscriptions, but no longer any duplicate. Load the list of forum subscriptions. No exception should be thrown! Go to the database and execute: insert into mdl_forum_subscriptions (userid, forum) values ( 2 , 5 ); You should get an error like: ERROR:  duplicate key value violates unique constraint "mdl_forusubs_usefor_uix" DETAIL:  Key (userid, forum)=( 2 , 5 ) already exists.
    • Affected Branches:
      MOODLE_32_STABLE, MOODLE_33_STABLE, MOODLE_34_STABLE
    • Fixed Branches:
      MOODLE_32_STABLE, MOODLE_33_STABLE
    • Pull Master Branch:
      MDL-59854-master

      Description

      In our production system I just found the following exception message, when displaying the newsletter forum of our site course:

      Did you remember to make the first column something unique in your call to get_records? Duplicate value '24028' found in column 'id'.

      • line 807 of /lib/dml/pgsql_native_moodle_database.php: call to debugging()
      • line 456 of /mod/forum/classes/subscriptions.php: call to pgsql_native_moodle_database->get_records_sql()
      • line 114 of /mod/forum/subscribers.php: call to mod_forum\subscriptions::fetch_subscribed_users()

      Indeed, for multiple forums of our production system, there exist duplicate subscriptions for the same users. They are not many, but they exist:

       

      select count( * ) ,userid, forum from mdl_forum_subscriptions group by userid, forum having count( * ) > 1;

       count | userid | forum
      ------++--------------
           2 |  52425 | 43775
           2 |  27997 | 51392
           2 |  24028 |     1
           2 |  29475 | 51392
           2 |  39667 | 36421
           2 |   6984 | 51392
           2 |  38646 | 48737
           2 |  38646 | 51740
           2 |  14616 | 51740
           2 |  21720 | 51392
           2 |  54188 | 51740
           2 |  54188 | 48737
           2 |  36801 | 51740
           2 |  46257 | 16885

      Looking at the subscribe function, there is a race condition, between checking if a subscription exists and creating a new one:

      public static function subscribe_user($userid, $forum, $context = null, $userrequest = false) {
          global $DB;
       
          if (self::is_subscribed($userid, $forum)) {
              return true;
          }
       
          $sub = new \stdClass();
          $sub->userid  = $userid;
          $sub->forum = $forum->id;
       
          $result = $DB->insert_record("forum_subscriptions", $sub);
      

       

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                4 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/Nov/17