Modify

Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#12198 closed defect (fixed)

Python recursion limit

Reported by: anonymous Owned by: Jun Omae
Priority: normal Component: DefaultCcPlugin
Severity: normal Keywords:
Cc: Trac Release:

Description

On a system with a lot of components (around 1000), the DefaultCC will cause a Python error (maximum recursion depth exceeded) on the component admin page.

It seems to be related to one of the Genshi functions (render) that DefaultCC uses.

Attachments (1)

t12198.diff (2.6 KB) - added by Jun Omae 9 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by Ryan J Ollos

Is there any useful info in the stack trace?

comment:2 in reply to:  1 Changed 10 years ago by anonymous

Replying to rjollos:

Is there any useful info in the stack trace?

Unfortunately it is on a system that I can't cut and paste from.

The Genshi version was 0.6-py2.5, and Trac was 0.11.6-py2.6, and I was running DefaultCC 0.2. Although I also observed the same issue on another system that runs the latest version of Trac/Genshi.

The issue is very easy to reproduce. Like I said you just need to add about 1k components (easiest is to just write a simple sqlite script). After that you will see the error when you go to the component admin panel.

According to the traceback, the original call was _dispatch_request from trac/web/main.py at line 450, which in turned called dispatch in the same file at line 227, and then render_template from trac/web/chrome.py at line 773. Then after a bunch of other calls, the __call__ function in /genshi/filters/transform.py will call itself recursively until it crash.

Last edited 9 years ago by Ryan J Ollos (previous) (diff)

comment:3 Changed 9 years ago by Ryan J Ollos

Here is a workaround that seems to be effective:

  • trunk/defaultcc/admin.py

     
    1010# you should have received as part of this distribution.
    1111#
    1212
     13from genshi import HTML
    1314from genshi.builder import tag
    1415from genshi.filters import Transformer
    1516from trac.core import Component, implements
     
    156157                                             '/tbody/tr[%d]' % (i + 1))
    157158                        stream |= filter.append(tag.td(default_cc,
    158159                                                       class_='defaultcc'))
     160                        # Avoid maximum recusion depth errors.
     161                        if i % 100 == 0:
     162                            stream = HTML(stream.render())
    159163                return stream
    160164
    161165        return stream

I'm not familiar with the internals of Genshi, so there may be a better way. Maybe someone else will have a better idea.

The page rendering is very slow though, and I suspect we can't do much better with Genshi. A workaround would be to modify the table using JavaScript, or perhaps optionally, not append Default CC to the table.

Are you still in need of a fix, and are you running Trac 0.11.6?

Changed 9 years ago by Jun Omae

Attachment: t12198.diff added

comment:4 Changed 9 years ago by Jun Omae

We shouldn't use many calls of Transformer() because each call of Transformer() needs one or more stack. CondFieldsGenshiPlugin has the same issue in #10342.

Instead, we should call Transformer.apply with custom transformation method only once. Otherwise, directly transform without Tranformer() like Chrome._strip_accesskeys(). See t12198.diff.

comment:5 Changed 9 years ago by Ryan J Ollos

Status: newaccepted

Thanks for the patch! The page renders very fast now. I'll try fixing #10342 as an exercise to better understand the patch :)

comment:6 Changed 9 years ago by Ryan J Ollos

In 14805:

0.3dev: Prevent recursion limit with many components.

Patch by Jun Omae. Refs #12198.

comment:7 Changed 9 years ago by Ryan J Ollos

In 14806:

0.4dev: Merged [14805] from 0.3 branch. Refs #12198.

comment:8 Changed 9 years ago by Ryan J Ollos

In 14807:

0.4dev: Require Trac 1.0 and later on trunk. Refs #12198.

comment:9 Changed 9 years ago by Ryan J Ollos

In 14808:

0.4dev: Place Default CC before Default column. Refs #12198.

comment:10 Changed 9 years ago by Ryan J Ollos

In 14809:

0.4dev: Removed unused imports. Refs #12198.

comment:11 Changed 9 years ago by Ryan J Ollos

Resolution: fixed
Status: acceptedclosed

comment:12 Changed 9 years ago by Ryan J Ollos

Owner: changed from Ryan J Ollos to Jun Omae

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.