Do explicit session garbage collection on cron

Created on 18 February 2019, about 5 years ago
Updated 20 February 2024, 11 days ago

Problem/Motivation

Make use of PHP's session_gc function which immediately executes session garbage collection. Currently we rely on PHP's innate unpredictable probability-based garbage collection. PHP recommends executing session garbage collection explicitly if possible, such as via a cronjob.

Executing session garbage collection in cron provides a reliable guarantee garbage collection is happening, we're able to explicitly log it, and there may be performance gains to running outside of normal requests. Especially if cron runners have improved resources.

Original summary

I found out that PHP 7.1 has a session_gc() function, and the documentation explicitly states that it is recommended to use that as a cronjob instead of probability-based GC, because that is, well, unpredictable :) Also one thing less that might run on user requests.

. No longer applicable.

Proposed resolution

Change default session.gc_probability settings to not run and call that function in system_cron().

πŸ“Œ Task
Status

Needs work

Version

11.0 πŸ”₯

Component
BaseΒ  β†’

Last updated 1 minute ago

Created by

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates : some issue and comment data are missing.

  • last update 8 months ago
    29,762 pass, 39 fail
  • πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

    Yeah, the active session thing is a bit awkward, but the documentation for session_gc() says that you should just call session_start() then, doing that with a session is started check and confirmed that this calls down into \Drupal\Core\Session\SessionHandler::gc().

    > I switched the session backend to redis and had a rude shock when I discovered that the default session.gc_maxlifetime is 24 minutes if you don't override it in services.yml. Yet my databases in environments that haven't done the switch yet have sessions in them that are 19 days old.

    The default setting is "gc_maxlifetime: 200000", that's ~2.3 days. And it confused my a lot initially as well, it's combined with the cookie expiration, which has an extra 0 and is ~23 days. The trick here is that by default, your session stays active up to 23 days as long as you visit the site at least once within those 2.3 days, then you extend your session expiration by another 2.3 days until eventually your cookie vanishes after 23 days even if you access it daily.

    There is no official redis session module yet, are you using the patch. Maybe there's a bug there, maybe it gets second/millisecond calculation wrong or something like that. I'm assuming that the redis logic uses built-in redis expiration and therefore doesn't need to do any garbage collection, redis handles that automatically.

  • Status changed to Needs work 8 months ago
  • πŸ‡¦πŸ‡ΊAustralia darvanen Sydney, Australia

    @Berdir, yes I'm using the patch.

    The default setting is "gc_maxlifetime: 200000", that's ~2.3 days. And it confused my a lot initially as well, it's combined with the cookie expiration, which has an extra 0 and is ~23 days. The trick here is that by default, your session stays active up to 23 days as long as you visit the site at least once within those 2.3 days, then you extend your session expiration by another 2.3 days until eventually your cookie vanishes after 23 days even if you access it daily.

    Thanks for the explanation, that makes sense. It also makes sense that the redis patch might be failing to convert seconds to milliseconds.

  • πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA

    +1 to this. I think this will improve security of sites. One consideration though is that now you can still have expired sessions lasting longer than they should. It all depends on how often cron runs on your site. But that's already the case with this 1% probability cleaner.

  • πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA

    The trick here is that by default, your session stays active up to 23 days as long as you visit the site at least once within those 2.3 days

    This is not true, at least with Drupal's default Drupal\Core\Session\SessionHandler:

      /**
       * {@inheritdoc}
       */
      #[\ReturnTypeWillChange]
      public function gc($lifetime) {
        // Be sure to adjust 'php_value session.gc_maxlifetime' to a large enough
        // value. For example, if you want user sessions to stay in your database
        // for three weeks before deleting them, you need to set gc_maxlifetime
        // to '1814400'. At that value, only after a user doesn't log in after
        // three weeks (1814400 seconds) will their session be removed.
        $this->connection->delete('sessions')
          ->condition('timestamp', REQUEST_TIME - $lifetime, '<')
          ->execute();
        return TRUE;
      }
    

    This examines the "timestamp" column in the sessions table to determine the "age" of a session. The problem is that the timestamp is only updated when session data changes, causing a session write. An authenticated just browsing the site will not cause the timestamp to be incremented.

    I think we also need to update the comment in default.services.yml to be accurate. It currently says this (emphasis mine):

    # Set session lifetime (in seconds), i.e. the grace period for session
    # data. Sessions are deleted by the session garbage collector after one
    # session lifetime has elapsed since the user's last visit. When a session
    # is deleted, authenticated users are logged out, and the contents of the
    # user's session is discarded.
    # @default 200000
    gc_maxlifetime: 200000

    I think at a time, this may have been true, maybe we always use to write the session at the end of every request? But we don't do that with our lazy sessions, which are only opened/written when there's something to write.

  • πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA

    Converted the patch to a MR and fixed an issue. The patch was running session_start(), but we ALSO need to run ->start() on the session object so that SessionManager can initialize the session configuration. Otherwise the values set in services.yml for the session lifetime aren't used.

    Leaving to Needs work since this will need a test

  • πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA
  • Pipeline finished with Failed
    19 days ago
    Total: 176s
    #93283
  • First commit to issue fork.
Production build https://api.contrib.social 0.61.6-2-g546bc20