Confirmed Opening extra.less gives fatal memory limit error

[CStrategies]

This just started recently, and yes I had a rather large extra.less with about 9000 lines. I was in the process of breaking it up into separate .less files, when suddenly I hit this error:

Fatal error: Allowed memory size of 536870912 bytes exhausted (tried to allocate 204800 bytes)

Now, my PHP mem limit is set to 512MB so I should not be having this problem even with a large .less file. I also have two separate styles with the same/similar large extra.less file, but I'm only getting the error on the one I've made changes to most frequently.

Therefore, I'm wondering if it has something to do with the history of changes that gets displayed on the 'edit template" page? Maybe one more change added just enough to the history to hit the limit? Is there any way to erase the history so I can test?

I found these threads:
https://xenforo.com/community/threads/fatal-error-allowed-memory-size-exhausted.178353/

https://xenforo.com/community/threa...hausted-tried-to-allocate-40960-bytes.177938/

https://xenforo.com/community/threa...-memory-limit-when-set-to-1-unlimited.178225/

https://xenforo.com/community/threads/open-extra-less-causes-error.217803/

But in general, users seem to have increased their PHP limit and moved on. I can't/won't increase further from 512MB, so I'm wondering if devs can get to the root of this bug or help me see where I might have missed a mem limit setting somewhere...

 

[CStrategies]

I increased the limit in XF.php to 1G, and that let me open the template so I can see what's going on. The template currently only has 900 lines, but at the bottom of the page there are 1200 rows of edit history, hence my guess that this XF bug is caused by edit history, especially with larger template files (as .less files tend to be).

You need to add something that decreases the amount of history information shown on this page when it reaches a certain threshold, or paginate it, or something like that.

 

[CStrategies]

Please move to bug reports.

 

[CStrategies]

@Chris D

The problem is here, in the XF\Admin\Controller\Template.php actionHistory method:

if ($template->History)
{
    $list = $template->History;
    $list = $list->toArray();
    arsort($list);
}

This code is pulling all historical revisions (i.e., every edit ever made) into PHP memory and re-sorting them. Since there is no limit to how many revisions are stored, a large or frequently edited template can cause the array to grow huge, eating up memory.

In my case, I've been using extra.less as a canvas of sorts for the past two months to test revisions to the styling of my site. At times, my edits have grown to thousands of lines, and I make dozens of saves per day.

The actionHistory method also doesn't respect the XF option templateHistoryLength, so changing this option value does nothing because the method will still fetch all history resulting in the fatal error. Recommend, at the very least, integrating templateHistoryLength into the fetch before any sorting actions. Best solution would be to place a per-page limiter in here.

 

[CStrategies]

Additionally, even if the actionHistory is corrected, this line from templateAddEdit also causes a memory error:

'hasHistory' => $template->exists() ? $template->History->count() : false,

 

[CStrategies]

After further testing, the root cause of the issue appears to be the TO::MANY relation of $template->History. Again, with indefinite records for each template, this relation will always cause a memory error if the history of edits gets too large.

 

[CStrategies]

Suggestion to replace if $template->History and similar:

$hasHistory = $this->finder('XF:TemplateHistory')
->where('type', $template->type)
->where('title', $template->title)
->where('style_id', $template->style_id)
->limit(1)
->fetch()
->count();

And in actionHistory, change this:

if ($template->History)
{
    $list = $template->History;
    $list = $list->toArray();
    arsort($list);
}

To this:

if ($hasHistory)
{
    $options = \XF::options();
    $templateHistoryLength = $options->templateHistoryLength;

    // limit in the DB, not in PHP
    $historyFinder = $template->getRelationFinder('History')
        ->order('edit_date', 'DESC')
        ->limit(50); //optional hard limit

    //respect changes user might make to the template history length option
    if ($templateHistoryLength > 0)
    {
        $cutoffDate = \XF::$time - ($templateHistoryLength * 86400);
        $historyFinder->where('edit_date', '>=', $cutoffDate);
    }

    $historyCollection = $historyFinder->fetch();
    $list = $historyCollection->toArray();
}

 

[Jeremy P]

The actionHistory method also doesn't respect the XF option templateHistoryLength, so changing this option value does nothing because the method will still fetch all history resulting in the fatal error. Recommend, at the very least, integrating templateHistoryLength into the fetch before any sorting actions. Best solution would be to place a per-page limiter in here.

The records are pruned from the table after the length (30 days by default) has elapsed via a daily cron. With that in mind, fetching all available history is sort of as designed, I just don't think we anticipated there being thousands of huge records for one template in that time period.

We can likely replace the history check with a simple count query, and paginate or have a hard limit on the overall history page.