Uploaded image for project: 'phpBB'
  1. phpBB
  2. PHPBB-8352

Incorrect timing logic in Cron function + acm_file.php

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • 3.0.6
    • 3.0.4
    • Caching (ACM)
    • None
    • PHP Environment: 5.2.6
      Database: MySQL(i) 5.1.25

      functions.php:page_footer() contains the test:

      if (method_exists($cache, 'tidy') && time() - $config['cache_gc'] > $config['cache_last_gc']) {$cron_type = 'tidy_cache'}

      which adds the image load "callback" to cron.php?cron_type=tidy_cache. cron.php then issues a $cache->tidy() subject to the same test condition. This then invoke the acm::tidy() method implemented by whichever ACM you've enabled, which by default (and the only supported module) is acm_file.php:tidy().

      This routine does an opendir and enumurates this list through a readdir for all sql_* and data_* excluding data_global. All the rest are @included one by one, which compiles all of these files in the cache setting the $expired and erroring on setting $this->sql_rowset[$query_id] (hence the @ prefix to the include); if the $expired flag is set then the file is removed. The data_global is similarly processed and then lastly the config entry 'cache_last_gc' is set. A broadly similar approach is adopted for the other ('last_queue_run', 'search_last_gc', 'warnings_last_gc', 'database_last_gc', 'session_last_gc') timers.

      The problem with this approach is that on an active forum, quite a few page views could be initiated between this time window between XXX_last_gc + XXX_gc threshold and the first cleanup process completing. Each of these will generate a cron.php request and other cleanup process. With an active forum you can half-dozen parallel clean-up processes running in parallel where only one is needed. In my case I have 10 forums on my server, where because of restart issues, these XXXX_last_gc times tend to sync up, so I can have 20 of these clean-up processes kicking off at the same time all trying to hit the same file system.

      We can't move this gc timestamp udpate into page_footer(), because there is no guaranty that the requesting client will honour the cron.php image request, and therefore clean up operations could be dropped. However, it makes a lot of sense doing it in the cron.php select dispatch before executing the relatively time consuming $cache->tidy(), etc. This way you ensure that this cleanup processing is entered once every XXXX_gc interval, but largely eliminating the redundant calls to cron and totally eliminating the duplicate calls to the cleanup processes and the races that result.

            Acyd Burn Meik Sievertsen [X] (Inactive)
            TerryE TerryE [X] (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:
              Resolved: