incubator-bloodhound-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary <gary.mar...@wandisco.com>
Subject Re: [Apache Bloodhound] #22: Infinite recursion error when initialising a plugin that extends itself
Date Wed, 04 Apr 2012 16:53:54 GMT
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

Mime
View raw message