You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bloodhound.apache.org by Apache Bloodhound <bl...@incubator.apache.org> on 2012/04/03 19:10:06 UTC
[Apache Bloodhound] #22: Infinite recursion error when initialising
a plugin that extends itself
#22: Infinite recursion error when initialising a plugin that extends itself
-----------------------+-------------------------------------
Reporter: gjm | Owner: gjm
Type: defect | Status: new
Priority: blocker | Milestone: RC1 for initial release
Component: trac core | Version:
Keywords: |
-----------------------+-------------------------------------
This bug might be considered as the combined fault of core trac and a
plugin that is implemented in this way. However, it is not necessarily
clear how a plugin should get around this and we should not see an
infinite recursion error when it happens.
As an example of the bug, install the [http://trac-
hacks.org/wiki/ThemeEnginePlugin ThemeEnginePlugin] - when enabled, there
is an infinite recursion:
{{{
Traceback (most recent call last):
File "trac/web/api.py", line 496, in send_error
data, 'text/html')
File "trac/web/chrome.py", line 982, in render_template
template = self.load_template(filename, method=method)
File "trac/web/chrome.py", line 942, in load_template
self.get_all_templates_dirs(), auto_reload=self.auto_reload,
File "trac/web/chrome.py", line 659, in get_all_templates_dirs
for provider in self.template_providers:
File "trac/core.py", line 78, in extensions
components = [component.compmgr[cls] for cls in classes]
File "trac/core.py", line 215, in __getitem__
component = cls(self)
File "trac/core.py", line 154, in __call__
self.__init__()
File "themeengine/admin.py", line 21, in __init__
self.system = ThemeEngineSystem(self.env)
File "trac/core.py", line 154, in __call__
self.__init__()
File "themeengine/api.py", line 65, in __init__
for provider in self.providers:
File "trac/core.py", line 78, in extensions
components = [component.compmgr[cls] for cls in classes]
File "trac/core.py", line 215, in __getitem__
component = cls(self)
File "trac/core.py", line 154, in __call__
self.__init__()
File "themeengine/api.py", line 65, in __init__
for provider in self.providers:
File "trac/core.py", line 78, in extensions
components = [component.compmgr[cls] for cls in classes]
File "trac/core.py", line 215, in __getitem__
component = cls(self)
File "trac/core.py", line 154, in __call__
self.__init__()
File "themeengine/api.py", line 65, in __init__
for provider in self.providers:
[..]
File "trac/core.py", line 154, in __call__
self.__init__()
File "themeengine/api.py", line 65, in __init__
for provider in self.providers:
File "trac/core.py", line 78, in extensions
components = [component.compmgr[cls] for cls in classes]
File "trac/core.py", line 215, in __getitem__
component = cls(self)
RuntimeError: maximum recursion depth exceeded while calling a Python
object
}}}
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/22>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #22: Infinite recursion error when
initialising a plugin that extends itself
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#22: Infinite recursion error when initialising a plugin that extends itself
------------------------+-------------------------------------
Reporter: gjm | Owner: gjm
Type: defect | Status: closed
Priority: blocker | Milestone: RC1 for initial release
Component: trac core | Version:
Resolution: fixed | Keywords:
------------------------+-------------------------------------
Changes (by gjm):
* status: accepted => closed
* resolution: => fixed
Comment:
Further work to continue in #27
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/22#comment:3>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #22: Infinite recursion error when
initialising a plugin that extends itself
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#22: Infinite recursion error when initialising a plugin that extends itself
------------------------+-------------------------------------
Reporter: gjm | Owner: gjm
Type: defect | Status: accepted
Priority: blocker | Milestone: RC1 for initial release
Component: trac core | Version:
Resolution: | Keywords:
------------------------+-------------------------------------
Changes (by gjm):
* status: new => accepted
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/22#comment:1>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #22: Infinite recursion error when
initialising a plugin that extends itself
Posted by Olemis Lang <ol...@gmail.com>.
On Wed, Apr 4, 2012 at 12:59 PM, Gary <ga...@wandisco.com> wrote:
>
> On 04/04/2012 06:08 PM, Ethan Jucovy wrote:
>>>
>>> Ah, thanks for pointing that out. I was about to write a ticket to get
>>> someone to get the theme engine to change the implementation.
>>>
>>> I don't particularly mind that a plugin wants to do a lot in their
>>> __init__ as long as they are prepared to deal with the potential error. As
>>> I think I already say, the only fault I see in Trac on this issue is that
>>> it doesn't deal with it quite helpfully enough. Basically I think it should
>>> survive the event to the extent of allowing the admin to disable the
>>> offending plugin from the admin interface.
>>
>>
>> Yeah, I just wanted to bring this up with a reference ASAP to make sure
>> it's easy to revisit the core patch later. I agree that Trac should deal
>> with this more helpfully. Surviving the exception would be nice; even just
>> catching the exception and re-raising it wrapped in some kind of
>> PluginInitializationException(plugin_name, exc) would probably be better
>> than the current behavior.
>>
>> FWIW this should probably apply to all plugin __init__ errors; the infinite
>> recursion error just happens to be harder to debug than most since
>> understanding it relies on some knowledge of Trac internals. But I notice
>> for example that code like the following in an active plugin is not handled
>> gracefully either:
>>
>> class BadPlugin(Component):
>> implements(IRequestFilter)
>> def __init__(self, *args, **kw):
>> raise Exception
>>
>> Best,
>> Ethan
>>
>
> OK, I think we are in agreement but you make some good points that I updated https://issues.apache.org/bloodhound/ticket/27 with.
>
> I'm almost tempted to adjust the patch to only apply to the ThemeEnginePlugin and investigate raising an appropriate exception as suggested. What does everyone else think?
>
0-
TBH
I don't like the decision of failing for this reason ... but ... I
also consider there's a chance to make things work a bit different in
ThemeEngine code .
--
Regards,
Olemis
Re: [Apache Bloodhound] #22: Infinite recursion error when initialising
a plugin that extends itself
Posted by Gary <ga...@wandisco.com>.
On 04/04/2012 06:08 PM, Ethan Jucovy wrote:
>> Ah, thanks for pointing that out. I was about to write a ticket to get
>> someone to get the theme engine to change the implementation.
>>
>> I don't particularly mind that a plugin wants to do a lot in their
>> __init__ as long as they are prepared to deal with the potential error. As
>> I think I already say, the only fault I see in Trac on this issue is that
>> it doesn't deal with it quite helpfully enough. Basically I think it should
>> survive the event to the extent of allowing the admin to disable the
>> offending plugin from the admin interface.
>
> Yeah, I just wanted to bring this up with a reference ASAP to make sure
> it's easy to revisit the core patch later. I agree that Trac should deal
> with this more helpfully. Surviving the exception would be nice; even just
> catching the exception and re-raising it wrapped in some kind of
> PluginInitializationException(plugin_name, exc) would probably be better
> than the current behavior.
>
> FWIW this should probably apply to all plugin __init__ errors; the infinite
> recursion error just happens to be harder to debug than most since
> understanding it relies on some knowledge of Trac internals. But I notice
> for example that code like the following in an active plugin is not handled
> gracefully either:
>
> class BadPlugin(Component):
> implements(IRequestFilter)
> def __init__(self, *args, **kw):
> raise Exception
>
> Best,
> Ethan
>
OK, I think we are in agreement but you make some good points that I
updated https://issues.apache.org/bloodhound/ticket/27 with.
I'm almost tempted to adjust the patch to only apply to the
ThemeEnginePlugin and investigate raising an appropriate exception as
suggested. What does everyone else think?
Cheers,
Gary
Re: [Apache Bloodhound] #22: Infinite recursion error when
initialising a plugin that extends itself
Posted by Ethan Jucovy <et...@gmail.com>.
>
> Ah, thanks for pointing that out. I was about to write a ticket to get
> someone to get the theme engine to change the implementation.
>
> I don't particularly mind that a plugin wants to do a lot in their
> __init__ as long as they are prepared to deal with the potential error. As
> I think I already say, the only fault I see in Trac on this issue is that
> it doesn't deal with it quite helpfully enough. Basically I think it should
> survive the event to the extent of allowing the admin to disable the
> offending plugin from the admin interface.
Yeah, I just wanted to bring this up with a reference ASAP to make sure
it's easy to revisit the core patch later. I agree that Trac should deal
with this more helpfully. Surviving the exception would be nice; even just
catching the exception and re-raising it wrapped in some kind of
PluginInitializationException(plugin_name, exc) would probably be better
than the current behavior.
FWIW this should probably apply to all plugin __init__ errors; the infinite
recursion error just happens to be harder to debug than most since
understanding it relies on some knowledge of Trac internals. But I notice
for example that code like the following in an active plugin is not handled
gracefully either:
class BadPlugin(Component):
implements(IRequestFilter)
def __init__(self, *args, **kw):
raise Exception
Best,
Ethan
Re: [Apache Bloodhound] #22: Infinite recursion error when initialising
a plugin that extends itself
Posted by Gary <ga...@wandisco.com>.
On 04/04/2012 05:43 PM, Ethan Jucovy wrote:
>> [
>> http://svn.apache.org/viewvc/incubator/bloodhound/trunk/trac/trac/core.py?view=diff&r1=1309469&r2=1309470&pathrev=1309470
>> r1309470] fixes by dynamically replacing the __init__ of components with
>> one that runs the original __init__ on the condition that it has not
>> already been run.
>>
>> I suspect what would be better in the long run would be to catch the
>> recursion error and turn it into something that can be reported but allows
>> Bloodhound to continue working. Plugins should probably keep track of
>> whether an instance has already been initialised.
>
> This issue was raised on trac-dev[1]. It was pointed out that the
> ThemeEngine plugin is incorrectly accessing its extension points in
> __init__, which it should not do:
>
> "A component constructor is called in a
> special way, as you've found out, and it should really do pretty
> much nothing. Example of valid use is to set a list to [], a
> hash to {}, or such. In trunk you could even use @lazy
> (http://trac.edgewall.org/changeset/10737) to help with such
> things. So doing anything lengthy is prohibited and the same
> for "re-entering" the component machinery."
>
> Perhaps Trac core should be throwing a more helpful error than infinite
> recursion, but the correct fix here is to make ThemeEngineSystem.__init__
> do less (e.g. ``self._info = None``) and populate its ``self.info`` dict
> lazily. There's no need for this __init__-replacing magic in core.
>
> [1]
> http://old.nabble.com/Create-new-ticket-vs-reopen--9418-(if-necessary--)-td33164546.html
>
Ah, thanks for pointing that out. I was about to write a ticket to get
someone to get the theme engine to change the implementation.
I don't particularly mind that a plugin wants to do a lot in their
__init__ as long as they are prepared to deal with the potential error.
As I think I already say, the only fault I see in Trac on this issue is
that it doesn't deal with it quite helpfully enough. Basically I think
it should survive the event to the extent of allowing the admin to
disable the offending plugin from the admin interface.
Cheers,
Gary
Re: [Apache Bloodhound] #22: Infinite recursion error when
initialising a plugin that extends itself
Posted by Ethan Jucovy <et...@gmail.com>.
>
> [
> http://svn.apache.org/viewvc/incubator/bloodhound/trunk/trac/trac/core.py?view=diff&r1=1309469&r2=1309470&pathrev=1309470
> r1309470] fixes by dynamically replacing the __init__ of components with
> one that runs the original __init__ on the condition that it has not
> already been run.
>
> I suspect what would be better in the long run would be to catch the
> recursion error and turn it into something that can be reported but allows
> Bloodhound to continue working. Plugins should probably keep track of
> whether an instance has already been initialised.
This issue was raised on trac-dev[1]. It was pointed out that the
ThemeEngine plugin is incorrectly accessing its extension points in
__init__, which it should not do:
"A component constructor is called in a
special way, as you've found out, and it should really do pretty
much nothing. Example of valid use is to set a list to [], a
hash to {}, or such. In trunk you could even use @lazy
(http://trac.edgewall.org/changeset/10737) to help with such
things. So doing anything lengthy is prohibited and the same
for "re-entering" the component machinery."
Perhaps Trac core should be throwing a more helpful error than infinite
recursion, but the correct fix here is to make ThemeEngineSystem.__init__
do less (e.g. ``self._info = None``) and populate its ``self.info`` dict
lazily. There's no need for this __init__-replacing magic in core.
[1]
http://old.nabble.com/Create-new-ticket-vs-reopen--9418-(if-necessary--)-td33164546.html
Re: [Apache Bloodhound] #22: Infinite recursion error when
initialising a plugin that extends itself
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#22: Infinite recursion error when initialising a plugin that extends itself
------------------------+-------------------------------------
Reporter: gjm | Owner: gjm
Type: defect | Status: accepted
Priority: blocker | Milestone: RC1 for initial release
Component: trac core | Version:
Resolution: | Keywords:
------------------------+-------------------------------------
Comment (by gjm):
[http://svn.apache.org/viewvc/incubator/bloodhound/trunk/trac/trac/core.py?view=diff&r1=1309469&r2=1309470&pathrev=1309470
r1309470] fixes by dynamically replacing the __init__ of components with
one that runs the original __init__ on the condition that it has not
already been run.
I suspect what would be better in the long run would be to catch the
recursion error and turn it into something that can be reported but allows
Bloodhound to continue working. Plugins should probably keep track of
whether an instance has already been initialised.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/22#comment:2>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker