Moodle
  1. Moodle
  2. MDL-39456

Cache file store fails if used within a destructor

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.3, 2.5, 2.6
    • Fix Version/s: 2.4.5, 2.5.1
    • Component/s: Caching
    • Labels:
    • Testing Instructions:
      Hide

      Unfortunately it is not possible to test this in core, and not possible to write unit tests for it (unit tests hold onto $CFG as well so you can't unset the global there).
      I'll attach a test script to this issue.
      First hit the script without this patch applied, you'll notice that you get lots of errors and its skrew up your moodledata/cache directory permissions.
      Next delete moodledata/cache directory - its messed up beyond repair now.
      Finally apply the patch and hit the script again.
      You'll notice no more error and no more skrewed up cache directory.

      Show
      Unfortunately it is not possible to test this in core, and not possible to write unit tests for it (unit tests hold onto $CFG as well so you can't unset the global there). I'll attach a test script to this issue. First hit the script without this patch applied, you'll notice that you get lots of errors and its skrew up your moodledata/cache directory permissions. Next delete moodledata/cache directory - its messed up beyond repair now. Finally apply the patch and hit the script again. You'll notice no more error and no more skrewed up cache directory.
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull 2.4 Branch:
      wip-MDL-39456-m24
    • Pull 2.5 Branch:
      wip-MDL-39456-m25
    • Pull Master Branch:
      wip-MDL-39456-m26
    • Rank:
      50118

      Description

      There is a bug within the cache file store if it is used within a destructor and the cache directory does not exist.
      This situation leads to make_writable_directory being called which results in an error being thrown because $CFG has already been destoryed.

      The solution is to hold onto a reference of $CFG within the file cache.
      PHP destorys objects when there are no more references to them, by holding on a reference of $CFG we can be sure it is available until the file store is no longer needed.

      This bug doesn't exist within core as we don't use caches within destructors, however it will arrive sooner rather than later if possible as it allows for destructors to make delayed writes to a cache and operate more closely to the session mechanisms.

      1. test.php
        0.9 kB
        Sam Hemelryk

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Alrighty, I've a solution up for this now.

          It involves holding onto a reference of the $CFG global within the file store cache and resetting it if need be when ensuring the directory exists.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Alrighty, I've a solution up for this now. It involves holding onto a reference of the $CFG global within the file store cache and resetting it if need be when ensuring the directory exists. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Attached test script - test.php

          Show
          Sam Hemelryk added a comment - Attached test script - test.php
          Hide
          Rajesh Taneja added a comment -

          Thanks Sam,

          Patch looks spot-on.
          Pushing it for integration.

          Show
          Rajesh Taneja added a comment - Thanks Sam, Patch looks spot-on. Pushing it for integration.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (24, 25 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (24, 25 & master), thanks!
          Hide
          Jérôme Mouneyrac added a comment - - edited

          Hi Sam, I could not pass the test instructions as expected (I tested only with integration 2.4), these are the exact steps I executed:
          1- updated my site to the commit before your patch
          2- hit test.php:

          Notice: Trying to get property of non-object in /Users/jerome/Sites/integration_24/lib/setuplib.php on line 1238 Warning: mkdir(): Permission denied in /Users/jerome/Sites/integration_24/lib/setuplib.php on line 1238 Notice: Trying to get property of non-object in /Users/jerome/Sites/integration_24/lib/moodlelib.php on line 6557 Fatal error: Uncaught exception 'coding_exception' with message 'debug/codingerror' in /Users/jerome/Sites/integration_24/cache/stores/file/lib.php:613 Stack trace: #0 /Users/jerome/Sites/integration_24/cache/stores/file/lib.php(414): cachestore_file->ensure_path_exists() #1 /Users/jerome/Sites/integration_24/cache/classes/loaders.php(500): cachestore_file->set('f528ff43ff5bb20...', 'two') #2 /Users/jerome/Sites/integration_24/cache/classes/loaders.php(1265): cache->set('one', 'two') #3 /Users/jerome/Sites/integration_24/test2.php(21): cache_application->set('one', 'two') #4 [internal function]: test->__destruct() #5 {main} thrown in /Users/jerome/Sites/integration_24/cache/stores/file/lib.php on line 613
          

          3- deleted the moodledata/cache
          4- ran the script, still same errors.
          5- refreshed the main page, it was a blank page full of permission denieds till the "memory is exhausted" error message.
          6- pull your changes
          7- refreshed the page, it was still displaying the same permission denieds. Refreshed/hit the test.php script, it was still displaying the same permission denieds.
          8- Additionally to the test instructions, I deleted the moodledata/cache again, and it worked.

          I tested on Mac OSX / Chrome.

          Show
          Jérôme Mouneyrac added a comment - - edited Hi Sam, I could not pass the test instructions as expected (I tested only with integration 2.4), these are the exact steps I executed: 1- updated my site to the commit before your patch 2- hit test.php: Notice: Trying to get property of non-object in /Users/jerome/Sites/integration_24/lib/setuplib.php on line 1238 Warning: mkdir(): Permission denied in /Users/jerome/Sites/integration_24/lib/setuplib.php on line 1238 Notice: Trying to get property of non-object in /Users/jerome/Sites/integration_24/lib/moodlelib.php on line 6557 Fatal error: Uncaught exception 'coding_exception' with message 'debug/codingerror' in /Users/jerome/Sites/integration_24/cache/stores/file/lib.php:613 Stack trace: #0 /Users/jerome/Sites/integration_24/cache/stores/file/lib.php(414): cachestore_file->ensure_path_exists() #1 /Users/jerome/Sites/integration_24/cache/classes/loaders.php(500): cachestore_file->set('f528ff43ff5bb20...', 'two') #2 /Users/jerome/Sites/integration_24/cache/classes/loaders.php(1265): cache->set('one', 'two') #3 /Users/jerome/Sites/integration_24/test2.php(21): cache_application->set('one', 'two') #4 [internal function]: test->__destruct() #5 {main} thrown in /Users/jerome/Sites/integration_24/cache/stores/file/lib.php on line 613 3- deleted the moodledata/cache 4- ran the script, still same errors. 5- refreshed the main page, it was a blank page full of permission denieds till the "memory is exhausted" error message. 6- pull your changes 7- refreshed the page, it was still displaying the same permission denieds. Refreshed/hit the test.php script, it was still displaying the same permission denieds. 8- Additionally to the test instructions, I deleted the moodledata/cache again, and it worked. I tested on Mac OSX / Chrome.
          Hide
          Sam Hemelryk added a comment -

          Aha I can confirm that the issue does exist on the 24 branch. I've tested 25 and master and things are fine there.
          I'll track down the 24 issue now.

          Show
          Sam Hemelryk added a comment - Aha I can confirm that the issue does exist on the 24 branch. I've tested 25 and master and things are fine there. I'll track down the 24 issue now.
          Hide
          Sam Hemelryk added a comment -

          Hmm I tracked it down to having an alternative cache in place.
          Jerome by chance are you using a different cache location, other than moodledata/cache?
          It could be that you've got another file cache store that you've configured at some point or it may be that you've defined $CFG->altcachedir.
          If thats the case you would need to delete that directory instead of moodledata/cache.
          You could test this by moving/deleting your muc config file.
          I found that I was using an cache store that was using another directory, when I deleted that directory things worked fine.
          Would you mind trying that for me.
          You could just run the following from your moodledata dir and repeat the test if you like:

          mv muc/config.php muc/config.backup.php

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hmm I tracked it down to having an alternative cache in place. Jerome by chance are you using a different cache location, other than moodledata/cache? It could be that you've got another file cache store that you've configured at some point or it may be that you've defined $CFG->altcachedir. If thats the case you would need to delete that directory instead of moodledata/cache. You could test this by moving/deleting your muc config file. I found that I was using an cache store that was using another directory, when I deleted that directory things worked fine. Would you mind trying that for me. You could just run the following from your moodledata dir and repeat the test if you like: mv muc/config.php muc/config.backup.php Cheers Sam
          Hide
          Eloy Lafuente (stronk7) added a comment -

          (sending it back to testing... thanks!)

          Show
          Eloy Lafuente (stronk7) added a comment - (sending it back to testing... thanks!)
          Hide
          Jérôme Mouneyrac added a comment -

          I did a full refresh of everything (git clone, db, folders...)

          I have a little issue to get the "its messed up beyond repair now." Everytime I delete the cache folder the site works again (but not the test.php).

          However I confirm that the rest of the steps work exactly as explained I'll pass it.

          Show
          Jérôme Mouneyrac added a comment - I did a full refresh of everything (git clone, db, folders...) I have a little issue to get the "its messed up beyond repair now." Everytime I delete the cache folder the site works again (but not the test.php). However I confirm that the rest of the steps work exactly as explained I'll pass it.
          Hide
          Jérôme Mouneyrac added a comment -

          Tested on 2.4 and 2.5 all work fine, thanks

          Show
          Jérôme Mouneyrac added a comment - Tested on 2.4 and 2.5 all work fine, thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many thanks, super-Jerome!

          Show
          Eloy Lafuente (stronk7) added a comment - Many thanks, super-Jerome!
          Hide
          Damyon Wiese added a comment -

          Thanks for your hard work. This issue has now been sent upstream and will soon be downloaded by millions of Moodlers!

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Thanks for your hard work. This issue has now been sent upstream and will soon be downloaded by millions of Moodlers! Regards, Damyon

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: