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

It's possible to hold a redis session lock forever

XMLWordPrintable

    • Any
    • MOODLE_310_STABLE, MOODLE_311_STABLE, MOODLE_39_STABLE
    • MOODLE_403_STABLE, MOODLE_404_STABLE
    • MDL-69684-M403_its-possible-to-hold-a-redis-session-lock-forever
    • MDL-69684-M404_its-possible-to-hold-a-redis-session-lock-forever
    • MDL-69684_its-possible-to-hold-a-redis-session-lock-forever
    • Hide

      Setup

      1. Install the PHP Redis extension (replace "X" as your server's PHP version, e.g. 8.0) 

        sudo apt update && sudo apt install -y phpX-redis

      2. Restart your web server.
      3. Confirm the Redis PHP extension is enabled: "php -i | grep redis" - You should see a bunch of lines related to redis.
      4. Install Redis. e.g. using Docker 

        docker run --name redis -p 6379:6379 -d redis

      5. Setup Redis as the session handler, e.g. copy the following to your config.php and set the expiry time ($CFG->session_redis_lock_expire) to an easily distinguishable value: 

        $CFG->session_handler_class = '\core\session\redis';
        		 
        $CFG->session_redis_host = '127.0.0.1';
        		 
        $CFG->session_redis_lock_expire = 42;

      Before applying the patch

      Manual Testing

      1. To see if Redis is being used, in a prompt on the server type the following command 

        redis-cli monitor

      2. Navigate around the site keeping an eye on the open terminal.
      3. Confirm that session locks are created with expiry time set to the same value as $CFG->session_redis_lock_expire set in your config.php.
      4. Confirm that you see a SETNX and a EXPIRE command for each created session lock, eg. 

        1654812249.749869 [0 127.0.0.1:56392] "SETNX" "0h0cvmc680rjmriqpralti003j.lock" "s:50:\"[pid 274359] bons.ai.tuwien.ac.at:/login/index.php\";"
        		 
        1654812249.749937 [0 127.0.0.1:56392] "EXPIRE" "0h0cvmc680rjmriqpralti003j.lock" "42"

      Note: the session lock is easily discernible by its session id AND the '.lock' extension.

      After applying the patch

      Manual Testing

      1. To see if Redis is being used, in a prompt on the server type the following command 

        redis-cli monitor

      2. Navigate around the site keeping an eye on the open terminal.
      3. Confirm that session locks are created with expiry time set to the same value as $CFG->session_redis_lock_expire set in your config.php.
      4. Confirm that you see only a SET command for each created session lock with the expiry matching the value set in $CFG->session_redis_lock_expire and nx defined, eg. 

        1654812196.987659 [0 127.0.0.1:46970] "SET" "1h43n2aq1ppcu7fvtsrskf6ef5.lock" "s:50:\"[pid 274359] bons.ai.tuwien.ac.at:/login/index.php\";" "ex" "42" "nx"

      Note: the session lock is easily discernible by its session id AND the '.lock' extension.

      Run the unit tests

      1. Add define('TEST_SESSION_REDIS_HOST', '127.0.0.1'); to your config.php
      2. Confirm the unit tests succeed. 

        vendor/bin/phpunit lib/tests/session_redis_test.php

      Show
      Setup Install the PHP Redis extension (replace " X " as your server's PHP version, e.g. 8.0) sudo apt update && sudo apt install -y phpX-redis Restart your web server. Confirm the Redis PHP extension is enabled: " php -i | grep redis " - You should see a bunch of lines related to redis. Install Redis. e.g. using Docker docker run --name redis -p 6379:6379 -d redis Setup Redis as the session handler, e.g. copy the following to your config.php and set the expiry time ( $CFG->session_redis_lock_expire ) to an easily distinguishable value: $CFG ->session_handler_class = '\core\session\redis' ; $CFG ->session_redis_host = '127.0.0.1' ; $CFG ->session_redis_lock_expire = 42; Before applying the patch Manual Testing To see if Redis is being used, in a prompt on the server type the following command redis-cli monitor Navigate around the site keeping an eye on the open terminal. Confirm that session locks are created with expiry time set to the same value as $CFG->session_redis_lock_expire set in your config.php . Confirm that you see a SETNX and a EXPIRE command for each created session lock, eg. 1654812249.749869 [0 127.0.0.1:56392] "SETNX" "0h0cvmc680rjmriqpralti003j.lock" "s:50:\"[pid 274359] bons.ai.tuwien.ac.at:/login/index.php\";" 1654812249.749937 [0 127.0.0.1:56392] "EXPIRE" "0h0cvmc680rjmriqpralti003j.lock" "42" Note: the session lock is easily discernible by its session id AND the '.lock' extension. After applying the patch Manual Testing To see if Redis is being used, in a prompt on the server type the following command redis-cli monitor Navigate around the site keeping an eye on the open terminal. Confirm that session locks are created with expiry time set to the same value as $CFG->session_redis_lock_expire set in your config.php . Confirm that you see only a SET command for each created session lock with the ex piry matching the value set in $CFG->session_redis_lock_expire and nx defined, eg. 1654812196.987659 [0 127.0.0.1:46970] "SET" "1h43n2aq1ppcu7fvtsrskf6ef5.lock" "s:50:\"[pid 274359] bons.ai.tuwien.ac.at:/login/index.php\";" "ex" "42" "nx" Note: the session lock is easily discernible by its session id AND the '.lock' extension. Run the unit tests Add define('TEST_SESSION_REDIS_HOST', '127.0.0.1'); to your config.php Confirm the unit tests succeed. vendor/bin/phpunit lib/tests/session_redis_test.php

      in /lib/classes/session/redis.php
      a session lock is acquired from redis with the following code:

      $haslock = $this->connection->setnx($lockkey, $whoami);
      		 
      if ($haslock) {
      		 
          $this->locks[$id] = $this->time() + $this->lockexpire;
      		 
          $this->connection->expire($lockkey, $this->lockexpire);
      		 
          return true;
      		 
      }

      Since the lock is acquired and expired in two separate calls it's possible for it to lock out a user indefinitely if moodle is taken down.

      It should use the atomic call to set the expiry at the same time.

      $haslock = $this->connection->set($lockkey, $whoami, ['nx', 'ex'=>$this->lockexpire]);
      		 
      if ($haslock) {
      		 
          $this->locks[$id] = $this->time() + $this->lockexpire;
      		 
          return true;
      		 
      }

      We've seen this happen once in production. I'm more than happy to make a pr for this issue.

       

        1. MDL-69684.png
          MDL-69684.png
          627 kB

            Daniel Ziegenberg Daniel Ziegenberg
            jamie@agilicus Jamie
            Brendan Heywood Brendan Heywood
            Huong Nguyen Huong Nguyen
            Ron Carl Alfon Yu Ron Carl Alfon Yu
            Votes:
            7 Vote for this issue
            Watchers:
            17 Start watching this issue

              Created:
              Updated:
              Resolved:

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

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