Redis 6.0 issues with session locking

[exussum12]

We are seeing issues of sessions disappearing and then coming back which started happening when moving to redis 6 from 5.3.7

This happens when the session lock is triggered

Expected behaviour

Session lock waits and then acquires a lock

Actual behaviour

Lock not acquired and the page load looks as if the session doesnt exist.

I'm seeing this behaviour on

• OS: Ubuntu 22.04
• Redis: 6.2.6
• PHP: 7.4
• phpredis: 6

Steps to reproduce, backtrace or example script

Adding to php.ini

session.save_handler = redis
session.save_path = "redis-node:6379"
redis.session.locking_enabled=1
redis.session.lock_retries=10

I've checked

• There is no similar issue from other users
• Issue isn't fixed in develop branch

Not fully sure what the issue is, but looks like nothing in dev

Looking at the diff between 5.3.7 and 6, the backoff strategies were added so im guessing its related to that, but I can't see the docs on adding to the php.ini for the session.

Happy to trial a solution.

[lauri-elevant]

Confirming, session_start() occasionally returns false, without any other messages.

[exussum12]

https://github.com/exussum12/redis-issue

Repo above reproduces the issue

Steps to reproduce.

1. clone above repo
2. docker-compose up
3. open a terminal tab
4. run curl 'http://localhost:8080/' -H 'User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/115.0' -H 'Cookie: PHPSESSID=880bba3b26274e101fcb145d9306fedf'
5. open another terminal tab
6. run command from 4 again

The second one will fail to return the session id.

Use exussum12/redis-issue@d9403bd

to fall back to another version and the above returns with a session id

Monitor mode shows no get is ever made

https://github.com/exussum12/redis-issue/blob/master/5.3.7

vs

https://github.com/exussum12/redis-issue/blob/master/6.0

[exussum12]

687a0b4#diff-d7896829bc47f45d33720c352a4b8aabd4dca447b6db9e2d4205be5b44ba5d9eR714

Seems to be the issue

5.3.7...6.0.0

it appears in the above changelog

[markitosgv]

Same issue here upgrading to redis-ext 6

[exussum12]

@michael-grunder any chance of taking a look at the attached pull request. Thanks

[markitosgv]

any updates on this? thanks!

[exussum12]

@yatsukhnenko any chance you can check the PR please?

[yatsukhnenko]

@exussum12 I think you should adjust redis.session.* settings instead of ignoring locking errors

[exussum12]

@yatsukhnenko

I do not really disagree on a high level. Its pushing the error to somewhere else.

Backwards compatibility / documentation
This is a breaking change which is not documented, the PR itself was looking at making better messages rather than changing behaviour.

This is not documented behaviour though. 5.3.7 works as the PR attached. 6 fails all together. Its actually a hard error to spot and imagine there are people running into this issue without knowing.

Other session handlers

I also do not think that session handlers should put their own rules ontop of session_start. Tweaking behaviour sure (to speed up lock checks for example) but not to change the behaviour.

Running the Repo above though other common session handlers gives the following results

No error
Files (default)
Memcache
Redis <= 5.3.7

Error
Memcached
Redis >= 6

So memcached was the only exception before Redis 6 happened.

Worth an ini opt in option for this major version and then going fully to this behaviour for next major version?

[yatsukhnenko]

the PR itself was looking at making better messages rather than changing behaviour.

@exussum12 Are you taking about #2386?

This is a breaking change which is not documented

I agree we need to pay more attention to documenting changes

Other session handlers

Files handler doesn't have the problem with locking because there's no timeout. You can simulate behaviour of files handler by setting very high value for redis.session.lock_wait_time

[exussum12]

Sorry I seen the commit message which looked like logging, the PR makes it clear that this was the intention #1908

The files handler I tested by setting the file to read only on the box. chattr +i session_file

This is about failing to get a write lock, I agree that an error should be returned but would also argue that the error should be when you try and write to the session.

Eg

session_start(); // fails to get write lock

echo $_SESSION["someVariable"];

No error - no writes are needed

session_start(); // fails to get write lock

$_SESSION["someVariable"] = "testing"; //  throws Exception - can't write to session

[Nebual]

This breaking change was fun to track down, and doesn't appear to be mentioned in the Changelog.

I would agree that there should be an ini setting like redis.session.lock_failure_readonly for whether to provide a readonly version of $_SESSION if the lock fails to be acquired (as was the < v6 behaviour), or if the whole session_start() call should fail as v6.0.0 does (which, if the application does not expect this, tends to look just like a new empty $_SESSION...).

Unfortunately as v6 has already been released for some time, it would be a BC break for v6.0.3 to switch back to v5's behaviour, so its too late for a simple revert like #2386 but that would be the desired behaviour if the ini was set.