Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-62891

var_export does not handle circular references and can't describe callables nicely

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.4.5, 3.5.1, 3.6
    • Fix Version/s: 3.4.6, 3.5.3
    • Component/s: Libraries, Logging
    • Labels:
    • Testing Instructions:
      Hide

      Testing difficulty: medium - requires source code modification and access to the server's error log

      1. Apply the following patch to simulate an exception happening during the shutdown:

      diff --git a/admin/tool/log/classes/log/manager.php b/admin/tool/log/classes/log/manager.php
      index 2b1d49107f..907cc0aa74 100644
      --- a/admin/tool/log/classes/log/manager.php
      +++ b/admin/tool/log/classes/log/manager.php
      @@ -190,6 +190,7 @@ class manager implements \core\log\manager {
            * this allows us to implement buffering of write operations.
            */
           public function dispose() {
      +        throw new \Exception('Accidents happen!');
               if ($this->stores) {
                   foreach ($this->stores as $store) {
                       $store->dispose();
      

      2. Load a Moodle page (does not matter which one, any request should make it).
      3. Check the error log of your web server.
      4. TEST: Confirm that there is a line describing where the exception happened and that it was ignored, like:

      Exception ignored in shutdown function tool_log\\log\\manager::dispose: Accidents happen!
      

      Show
      Testing difficulty: medium - requires source code modification and access to the server's error log 1. Apply the following patch to simulate an exception happening during the shutdown: diff --git a/admin/tool/log/classes/log/manager.php b/admin/tool/log/classes/log/manager.php index 2b1d49107f..907cc0aa74 100644 --- a/admin/tool/log/classes/log/manager.php +++ b/admin/tool/log/classes/log/manager.php @@ -190,6 +190,7 @@ class manager implements \core\log\manager { * this allows us to implement buffering of write operations. */ public function dispose() { + throw new \Exception('Accidents happen!'); if ($this->stores) { foreach ($this->stores as $store) { $store->dispose(); 2. Load a Moodle page (does not matter which one, any request should make it). 3. Check the error log of your web server. 4. TEST: Confirm that there is a line describing where the exception happened and that it was ignored, like: Exception ignored in shutdown function tool_log\\log\\manager::dispose: Accidents happen!
    • Affected Branches:
      MOODLE_34_STABLE, MOODLE_35_STABLE, MOODLE_36_STABLE
    • Fixed Branches:
      MOODLE_34_STABLE, MOODLE_35_STABLE
    • Pull from Repository:
    • Pull 3.5 Branch:
      MDL-62891-35-callable_name
    • Pull Master Branch:
      MDL-62891-master-callable_name

      Description

      We are seeing the following error on a community site after having it upgraded to 3.5.1:

      Warning: var_export does not handle circular references in /opt/app/lib/classes/shutdown_manager.php on line 83
      

      As can be seen in the code, this is caused by

      error_log('Exception ignored in shutdown function '.var_export($callback, true).':'.$e->getMessage());
      

      The callback can contain circular references. In our case, this is caused by tool_log\log\manager that registers its own static method dispose() as a callback.

      We should think of a safer / better way to extract the class name and method name in these cases so that we have something like this in error logs:

      Exception ignored in shutdown function tool_log\log\manager::dispose()
      

      and not the rubbish as found e.g. in https://tracker.moodle.org/browse/MDL-46904?focusedCommentId=314863&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-314863

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                mudrd8mz David Mudrák (@mudrd8mz)
                Reporter:
                mudrd8mz David Mudrák (@mudrd8mz)
                Peer reviewer:
                Neill Magill
                Integrator:
                Eloy Lafuente (stronk7)
                Tester:
                Eloy Lafuente (stronk7)
                Participants:
                Component watchers:
                Amaia Anabitarte, Carlos Escobedo, Ferran Recio, Sara Arjona (@sarjona), Víctor Déniz Falcón, Andrew Nicols, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Nov/18