[MDL-68696] Read only sessions option throw exception with tool_mobile_get_public_config mobile when using redis sessions - Moodle Tracker

XML

Word

Printable

Details

Type: Bug
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 3.9
Fix Version/s: 3.9
Component/s: Authentication, Performance
Labels:
- CatalystIT
- ci
- partner
- triaged

Affected Branches:
MOODLE_39_STABLE
Fixed Branches:
MOODLE_39_STABLE
Pull from Repository:
https://github.com/Agilicus/moodle
Pull Master Branch:
read-only-session
Pull Master Diff URL:
https://github.com/moodle/moodle/compare/master...Agilicus:read-only-session

Testing Instructions:

Hide

Setup Redis server: https://docs.moodle.org/en/Redis_cache_store
- Install redis-server
- Install Redis PHP driver
- Configure Redis in Moodle

Configure Redis as the session handler:

$CFG->session_redis_host = '127.0.0.1';

$CFG->session_redis_port = 6379;

$CFG->session_redis_acquire_lock_timeout = 5;

$CFG->session_handler_class = '\core\session\redis';

Turn on read only sessions:

$CFG->enable_read_only_sessions = true;

Run this web service (you'll need to replace moodle.local by your local Moodle URL):

curl -v -X POST 'http://moodle.local/lib/ajax/service.php?info=tool_mobile_get_public_config' -d '[{"siteUrl": "http://moodle.local", "index": 0, "methodname":"tool_mobile_get_public_config", "args":{}}]'

Confirm you don't get an exception

Show

Setup Redis server: https://docs.moodle.org/en/Redis_cache_store Install redis-server Install Redis PHP driver Configure Redis in Moodle Configure Redis as the session handler: $CFG->session_redis_host = '127.0.0.1'; $CFG->session_redis_port = 6379; $CFG->session_redis_acquire_lock_timeout = 5; $CFG->session_handler_class = '\core\session\redis'; Turn on read only sessions: $CFG->enable_read_only_sessions = true; Run this web service (you'll need to replace moodle.local by your local Moodle URL): curl -v -X POST 'http://moodle.local/lib/ajax/service.php?info=tool_mobile_get_public_config' -d '[{"siteUrl": "http://moodle.local", "index": 0, "methodname":"tool_mobile_get_public_config", "args":{}}]' Confirm you don't get an exception

Description

I'm not too sure on this as a patch, but it resolves the issue.

On startup, the mobile app does a POST as:

curl -X POST 'http://localhost/lib/ajax/service.php?info=tool_mobile_get_public_config&XDEBUG_SESSION_START=VSCODE' -d '[\{"siteUrl": "http://localhost", "index": 0, "methodname":"tool_mobile_get_public_config", "args":{}}]'

If read-only sessions and redis are enabled, this hits the handler_read() function.

In there, if finds there is not yet a session in redis, then, if write-lock is needed, returns '' else returns false.

Since false is returned, we throw later on.

diff --git a/lib/classes/session/redis.php b/lib/classes/session/redis.php

index ff11a66129..f2da4525b0 100644

--- a/lib/classes/session/redis.php

+++ b/lib/classes/session/redis.php

@@ -257,8 +257,10 @@ class redis extends handler {

                 $this->lock_session($id);

             $sessiondata = $this->connection->get($id);

-            if ($sessiondata === false && $this->requires_write_lock()) {

-                $this->unlock_session($id);

+            if ($sessiondata === false) {

+                if ($this->requires_write_lock()) {

+                    $this->unlock_session($id);

+                }

                 return '';

             $this->connection->expire($id, $this->timeout);

Attachments

- Sort By Name
- Sort By Date
- Ascending
- Descending
- Thumbnails
- List
- Download All

image-2020-05-19-11-09-35-737.png
174 kB
19/May/20 11:09 AM

Issue Links

has a non-specific relationship to

MDL-58018 Reduce session lock contention with opt-in READ_ONLY_SESSION

Closed

Activity

People

Assignee:: Don Bowman

Reporter:: Don Bowman

Peer reviewer:: Brendan Heywood

Integrator:: Sara Arjona (@sarjona)

Tester:: Janelle Barcega

Participants:: Brendan Heywood, CiBoT, Don Bowman, Eloy Lafuente (stronk7), Janelle Barcega, Matteo Scaramuccia, Sara Arjona (@sarjona)

Component watchers:: David Woloszyn, Huong Nguyen, Jake Dallimore, Meirza, Michael Hawkins, Raquel Ortega, Safat Shahin, Stevani Andolo, Matteo Scaramuccia, David Woloszyn, Huong Nguyen, Jake Dallimore, Meirza, Michael Hawkins, Raquel Ortega, Safat Shahin, Stevani Andolo

Votes:: 3 Vote for this issue

Watchers:: 6 Start watching this issue

Dates

Created:: 13/May/20 7:29 AM

Updated:: 20/May/20 12:43 AM

Resolved:: 20/May/20 12:43 AM

Fix Release Date:: 15/Jun/20

Time Tracking

Estimated:

Not Specified

Remaining:

Logged:

2h 20m