@julienmoumne opened this Issue on July 8th 2012 Member

This is a follow up of #3191.

Timo pointed out a core issue in the way Piwik_DataTable identifiers are handled by Piwik_DataTable_Manager.

In Piwik_DataTable_Row->__destruct(), the sub-DataTable associated to the current Piwik_DataTable_Row is loaded and deleted.

The identifier used to load and delete the sub-DataTable is the one stored in $this->c[self::DATATABLE_ASSOCIATED].

This identifier is used in Piwik_DataTable_Manager->getTable($idTable) to retrieve the Piwik_DataTable instance from the Piwik_DataTable_Manager->tables variable.

Piwik_DataTable_Manager->tables array identifiers are managed with the Piwik_DataTable_Manager->nextTableId variable. It is incremented in Piwik_DataTable_Manager->addTable($table) each time a table is added.

When expanded = 1, the value of Piwik_DataTable_Manager->nextTableId is fed back to Piwik_DataTable_Row->c[in [https://github.com/piwik/piwik/blob/master/core/Archive/Single.php#L353 Piwik_Archive_Single->loadSubDataTables()](self::DATATABLE_ASSOCIATED]) :

// and update the subtableID so that it matches the newly instanciated table

In some legitimate cases, for example, when expanded = 0, sub-DataTables are not retrieved from the database. They are therefore not added to the Piwik_DataTable_Manager->tables array.

In those cases, the identifier stored in Piwik_DataTable_Row->c[self::DATATABLE_ASSOCIATED] is equal to the number appended to the name column in the archive tables. Example :

Referers_searchEngineByKeyword_100

Unfortunately, as explained above, this identifier is used to load and delete a DataTable instance stored in the Piwik_DataTable_Manager->tables array. In this case, this Piwik_DataTable instance (if present) is not related to the Piwik_DataTable_Row being destructed.

This is the reason why we have long nested stack traces when destructing Piwik_DataTable instances.


We experienced this behavior in #3191.

I also experience this behavior on my box when running EcommerceOrderWithItems.test.php after I switched the two faulty looking lines in Piwik_DataTable_Manager->deleteTable($id)

from

105             $this->setTableDeleted($id);
106             destroy($this->tables[$id]);

to

105             destroy($this->tables[$id]);
106             $this->setTableDeleted($id);

To make the test pass I had to set

xdebug.max_nesting_level = 105

The stack trace below is a good example of the issue at hand :

67  5.3957  18663968    Piwik_DataTable_Row->__destruct( )  ../Common.php:1888
68  5.3957  18663968    Piwik_DataTable_Manager->deleteTable( ) ../Row.php:98
69  5.3957  18663968    destroy( )  ../Manager.php:105
70  5.3957  18664012    Piwik_DataTable->__destruct( )  ../Common.php:1888
71  5.3958  18664640    destroy( )  ../DataTable.php:274

Line 67 : Destruction of a Piwik_DataTable_Row instance

Line 70 : Destruction of a Piwik_DataTable instance

Lines 68, 69, 70 and 71 should not have occurred as EcommerceOrderWithItems.test.php does not call any API with expanded = 1.


This issue is hard to fix because when expanded = 1, not all sub-DataTables are retrieved from memory and loaded into the Piwik_DataTable_Manager->tables array.

@mattab commented on July 31st 2012 Member

Thank you for the great analysis, it was really helpful in my understanding of the issue. I mark as duplicate of: #3207 where this bug was fixed. Thanks again!!

@julienmoumne commented on August 2nd 2012 Member

(In [6652]) refs #3263 unit tests

@julienmoumne commented on August 2nd 2012 Member

(In [6653]) refs #3263 simplifying unit tests and adding required annotations

@julienmoumne commented on August 4th 2012 Member

(In [6669]) refs #3263 fixing unit tests and adding some more

@mattab commented on August 4th 2012 Member

Kuddos for spotting the issue with serialize() leaving the object in the wrong state!!

This Issue was closed on August 4th 2012
Powered by GitHub Issue Mirror