Avoid locking the php session

[juliusknorr]

Slightly different approach about has the same target as #29356 to reduce the time that request might need to wait for locked sessions.

We already try to write and close the session early but on multiple concurrent requests that is still not fast enough to not have noticeable wait time on the lock. To my knowledge we don't write anything into the session that would be critical in terms of using already read value that has been updated by another request, so I think this approach should be fine from the potential race condition perspective, especially with reopening the session we would read again.

Impact on page load can be tested by adding a sleep for specific routes:

diff --git a/lib/private/Session/Internal.php b/lib/private/Session/Internal.php
index 1dfec8d71e..db9002f8be 100644
--- a/lib/private/Session/Internal.php
+++ b/lib/private/Session/Internal.php
@@ -233,5 +233,8 @@ class Internal extends Session {
                        $sessionParams['read_and_close'] = $readAndClose;
                }
                $this->invoke('session_start', [$sessionParams], $silence);
+               if (strpos(\OC::$server->getRequest()->getRequestUri(), '/ocs/v2.php/search/providers') === 0) {
+                       sleep(10);
+               }
        }
 }

Currently every request writes to the session

server/lib/base.php

Line 455 in d42911c

$session->set('LAST_ACTIVITY', time());

so we always need to at least open once for reading and that write, so second commit implements what was described in #29356 but requires that we do no longer invalidate sessions on the Nextcloud side, but rely on the PHP garbage collection for it.

I decided to go for a config flag for this, as for most setups having a session timeout that is happening at the exact second might not be really needed. If that is not used, PHP may clean up the session at a later point depending on the session garbage collection configuration. If using redis, the session lifetime will be set anyways on redis, so redis would take care of the deletion using configured key eviction policy.

[juliusknorr]

Needs extra care for talk which seems to have issues with no session locking and there this PR could then lead to similar problems where the one session might be lost if the other one writes

[PVince81]

any connection with #31286 ?

[juliusknorr]

Tested with the Talk edge case of joining a public room with a different user and seems to work.

Further cases to test:

• Files mobile client Android
• Talk mobile client Android
• Talk call and chat between user 1 (web), user 2 (android), public guest link (web)
• Log and see how many reopening are happening

[juliusknorr]

Some screenshots for comparison with the sleep added on master and this branch:

[PVince81]

👍

[PVince81]

please rebase/fixup

[violethaze74]

👍🏻