Create new ticket vs reopen #9418 (if necessary ;)
Olemis Lang
My comment posted to #9580 @ t.h.o [1]_ makes me wonder whether it is
convenient to reopen #9418 @ t.e.o
I'll try to be more precise and add further details later , but I just
wanted to know if this is better discussed here in the ML or in the
ticket itself (i.e. #9418 @ t.e.o.)
Thnx in advance !
.. [1] Recursion after installing in Trac 0.13 (dev) <= ThemeEnginePlugin
(http://trac-hacks.org/ticket/9580#comment:1)
--
Regards,
Olemis
Facebook => http://www.facebook.com/olemis
Twitter => http://www.twitter.com/olemislc (@olemislc)
Blog ES => http://simelo-es.blogspot.com
Blog EN => http://simelo-en.blogspot.com
Quora => http://www.quora.com/olemis
Youtube => http://youtube.com/user/greatsoftw
Featured article : Identificando números primos con expresión regular en Perl
http://feedproxy.google.com/~r/simelo-news/~3/BHr859OSndo/identificando-numeros-primos-con.html
Tweet: parsear RT @yudivian Cual es el equivalente correcto en el
español de "parsear"
Follow @olemislc Reply Retweet 15:58 Jan-18
Get this email app!
Get a signature like this. CLICK HERE.
Christian Boos
> Hi all !
>
> My comment posted to #9580 @ t.h.o [1]_ makes me wonder whether it is
> convenient to reopen #9418 @ t.e.o
> I'll try to be more precise and add further details later , but I just
> wanted to know if this is better discussed here in the ML or in the
> ticket itself (i.e. #9418 @ t.e.o.)
>
> Thnx in advance !
>
> .. [1] Recursion after installing in Trac 0.13 (dev)<= ThemeEnginePlugin
> (http://trac-hacks.org/ticket/9580#comment:1)
>
I don't see the recursion... care to post a traceback (here)?
-- Christian
Olemis Lang
On Wed, Jan 18, 2012 at 5:13 PM, Olemis Lang <ole...@gmail.com> wrote:
> Hi all !
>
> My comment posted to #9580 @ t.h.o [1]_ makes me wonder whether it is
> convenient to reopen #9418 @ t.e.o
> I'll try to be more precise and add further details later , but I just
> wanted to know if this is better discussed here in the ML or in the
> ticket itself (i.e. #9418 @ t.e.o.)
>
«change your code rather than ComponentMeta.__call__ »
;)
I look forward to your reply .
>
> .. [1] Recursion after installing in Trac 0.13 (dev) <= ThemeEnginePlugin
> (http://trac-hacks.org/ticket/9580#comment:1)
>
--
Regards,
Olemis
Facebook => http://www.facebook.com/olemis
Twitter => http://www.twitter.com/olemislc (@olemislc)
Blog ES => http://simelo-es.blogspot.com
Blog EN => http://simelo-en.blogspot.com
Quora => http://www.quora.com/olemis
Youtube => http://youtube.com/user/greatsoftw
Gary
That is interesting. In tracking down the Trac code changes associated
with the recursion error, I did not consider the possibility that there
might be justification for the component containing itself through that
method. I believe that the patch I suggested has a bug in the case of
the selected themes being disabled.
Anyway, as Olemis has not replied yet and I have just spotted this
thread, I thought I would try to replicate the appropriate traceback for
the recursion on installation of the unpatched plugin. Sorry I neglected
to put this in the trac-hacks ticket that Olemis referenced (traceback
replicated on r10876 from trunk):
Traceback (most recent call last):
File "/home/trac/projects/trac/trac-trunk/trac/web/api.py", line 498, in send_error
data, 'text/html')
File "/home/trac/projects/trac/trac-trunk/trac/web/chrome.py", line 982, in render_template
template = self.load_template(filename, method=method)
File "/home/trac/projects/trac/trac-trunk/trac/web/chrome.py", line 942, in load_template
self.get_all_templates_dirs(), auto_reload=self.auto_reload,
File "/home/trac/projects/trac/trac-trunk/trac/web/chrome.py", line 653, in get_all_templates_dirs
for provider in self.template_providers:
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 78, in extensions
components = [component.compmgr[cls] for cls in classes]
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 199, in __getitem__
component = cls(self)
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 138, in __call__
self.__init__()
File "build/bdist.linux-x86_64/egg/themeengine/admin.py", line 21, in __init__
self.system = ThemeEngineSystem(self.env)
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 138, in __call__
self.__init__()
File "build/bdist.linux-x86_64/egg/themeengine/api.py", line 65, in __init__
for provider in self.providers:
[snip]
File "build/bdist.linux-x86_64/egg/themeengine/api.py", line 65, in __init__
for provider in self.providers:
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 78, in extensions
components = [component.compmgr[cls] for cls in classes]
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 199, in __getitem__
component = cls(self)
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 138, in __call__
self.__init__()
File "build/bdist.linux-x86_64/egg/themeengine/api.py", line 65, in __init__
for provider in self.providers:
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 78, in extensions
components = [component.compmgr[cls] for cls in classes]
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 199, in __getitem__
component = cls(self)
File "/home/trac/projects/trac/trac-trunk/trac/core.py", line 138, in __call__
self.__init__()
File "build/bdist.linux-x86_64/egg/themeengine/api.py", line 65, in __init__
for provider in self.providers:
RuntimeError: maximum recursion depth exceeded while calling a Python object
Thanks for taking an interest :)
Cheers,
Gary
--
Best wishes,
Gary Martin
Lead Developer
WANdisco, Inc.
Remy Blank
> Traceback (most recent call last):
> File "/home/trac/projects/trac/trac-trunk/trac/web/api.py", line 498, in send_error
> data, 'text/html')
This looks like a secondary error that happens while trying to render
the error page. On that page, we have the list of components, and that
one requires the component to be initialized. Which is kind of difficult
if the initial error happens during initialization...
I assume you got this traceback from your trac.log. Is it preceded by
another traceback? If so, that's the actual error you are trying to fix.
Please post that traceback.
-- Remy
Olemis Lang
Sorry for the delay . I was trying to make some research to determine
the cause of this issue
On Wed, Jan 18, 2012 at 7:59 PM, Remy Blank <remy....@pobox.com> wrote:
> Gary wrote:
>> Traceback (most recent call last):
>> File "/home/trac/projects/trac/trac-trunk/trac/web/api.py", line 498, in send_error
>> data, 'text/html')
>
> This looks like a secondary error that happens while trying to render
> the error page. On that page, we have the list of components, and that
> one requires the component to be initialized. Which is kind of difficult
> if the initial error happens during initialization...
>
No, that happens when ThemeEngineSystem instance is to be initialized
(i.e. it is installed and enabled) .
Nonetheless the issue is not limited to that plugin IMO .
Besides I confirmed that r10295 is causing this as everything works as
expected in r10294 .
I provide further details based on the research I made
;)
I updated my working copy to r10294 (immediately before committing for the
first time what was discussed in #9418 ;) and Trac website
(i.e. home page with no theme applied, and everything else ;) works ok .
At that moment :
{{{
#!sh
$ hg identify
3902453643bf (trunk) svn-10294
}}}
When moving forward to r10295 similar failure happens ... and I say
**similar** because traceback looks a bit different due to subsequent
changes committed in r10298 and r10300 . To be more precise
{{{
#!sh
$ hg identify
8dda5d3eb23d (trunk) svn-10295
}}}
> I assume you got this traceback from your trac.log. Is it preceded by
> another traceback? If so, that's the actual error you are trying to fix.
> Please post that traceback.
>
Nope ... tb is rendered rather than page contents ; just like it
happens with unhandled Trac errors .
:-/
Everything starts with this
{{{
Traceback (most recent call last):
File "/path/to/trac/hg/mirror/trac-mq/trac/web/api.py", line 496, in
send_error
data, 'text/html')
File "/path/to/trac/hg/mirror/trac-mq/trac/web/chrome.py", line 982,
in render_template
template = self.load_template(filename, method=method)
File "/path/to/trac/hg/mirror/trac-mq/trac/web/chrome.py", line 942,
in load_template
self.get_all_templates_dirs(), auto_reload=self.auto_reload,
File "/path/to/trac/hg/mirror/trac-mq/trac/web/chrome.py", line 653,
in get_all_templates_dirs
for provider in self.template_providers:
File "/path/to/trac/hg/mirror/trac-mq/trac/core.py", line 78, in extensions
components = [component.compmgr[cls] for cls in classes]
File "/path/to/trac/hg/mirror/trac-mq/trac/core.py", line 199, in __getitem__
component = cls(self)
File "/path/to/trac/hg/mirror/trac-mq/trac/core.py", line 138, in __call__
self.__init__()
}}}
... but honestly snippet above is not really relevant (if you need me to
explain why this is not relevant and what's happening under the hood please
let me know , I'll do that asaic ;) . You can start by checking
from line below (i.e. you can invoke `theme_eng = ThemeEngineSystem(env)` from
the command line python interpreter and you'll get the same result ;)
{{{
File "/path/to/trac/0.13/env/plugins/TracThemeEngine-2.0-py2.6.egg/themeengine/admin.py",
line 21, in __init__
self.system = ThemeEngineSystem(self.env)
}}}
... immediately after doing so the following calls are repeated as
part of the infinite loop ...
{{{
File "/path/to/trac/hg/mirror/trac-mq/trac/core.py", line 138, in __call__
self.__init__()
File "/path/to/trac/0.13/env/plugins/TracThemeEngine-2.0-py2.6.egg/themeengine/api.py",
line 63, in __init__
for provider in self.providers:
File "/path/to/trac/hg/mirror/trac-mq/trac/core.py", line 78, in extensions
components = [component.compmgr[cls] for cls in classes]
File "/path/to/trac/hg/mirror/trac-mq/trac/core.py", line 199, in __getitem__
component = cls(self)
}}}
... which means that after this you get another similar sequence (...
I think I should not repeat the same, I'm just hoping everybody gets
the idea ;)
I'm writing a patch containing a unit test with a simplified scenario
illustrating the barely minimal context for this to happen . The root
cause for infinite loop is that after r10295 instances are registered
in ComponentManager's «pool» after calling __init__ method (#9418
records the whole discussion ;)
PS: Like I mentioned before if you need me to
explain what's happening under the hood please
let me know , I'll do it asap ;)
Thanks for your promptly response
--
Regards,
Olemis
Facebook => http://www.facebook.com/olemis
Twitter => http://www.twitter.com/olemislc (@olemislc)
Blog ES => http://simelo-es.blogspot.com
Blog EN => http://simelo-en.blogspot.com
Quora => http://www.quora.com/olemis
Youtube => http://youtube.com/user/greatsoftw
Featured article : Identificando números primos con expresión regular en Perl
http://feedproxy.google.com/~r/simelo-news/~3/BHr859OSndo/identificando-numeros-primos-con.html
Tweet: check it out ! ;) RT @arielmcorg Lista la aplicación oficial de
Wikipedia para Android http://t.co/pdwfBr3x #fb
Follow @olemislc Reply Retweet 07:56 Jan-19
Olemis Lang
[...]
>
> I'm writing a patch containing a unit test with a simplified scenario
> illustrating the barely minimal context for this to happen .
Nonetheless I'm experiencing this issue when I run Trac test suite .
http://paste.pocoo.org/show/537319/
I'll be connected to #trac @ irc.freenode.net , just in case you
prefer suggest anything in there ;)
Jun Omae
On Fri, Jan 20, 2012 at 00:44, Olemis Lang <ole...@gmail.com> wrote:
> I'm writing a patch containing a unit test with a simplified scenario
> illustrating the barely minimal context for this to happen . The root
> cause for infinite loop is that after r10295 instances are registered
> in ComponentManager's «pool» after calling __init__ method (#9418
> records the whole discussion ;)
I can reproduce the issue and understand the condition. During a
component is initializing, accessing extension points of the component
causes infinite loop.
Solution 1.
Detect and refuse accessing the extension points during initializing
at `ExtensionPoint.extensions`.
diff --git a/trac/core.py b/trac/core.py
index 7c03577..e3594bf 100644
--- a/trac/core.py
+++ b/trac/core.py
@@ -75,6 +75,11 @@ class ExtensionPoint(property):
extension point interface.
"""
classes = ComponentMeta._registry.get(self.interface, ())
+ for cls in classes:
+ if cls not in component.compmgr and isinstance(component, cls):
+ raise RuntimeError('Cannot access extension points of the '
+ 'component during a component is '
+ 'initializing')
components = [component.compmgr[cls] for cls in classes]
return [c for c in components if c]
Solution 2.
When detecting accessing the extension points, use `component` variable
instead of `component.compmgr[cls]` at `ExtensionPoint.extensions`.
diff --git a/trac/core.py b/trac/core.py
index 7c03577..13a355d 100644
--- a/trac/core.py
+++ b/trac/core.py
@@ -75,7 +75,11 @@ class ExtensionPoint(property):
extension point interface.
"""
classes = ComponentMeta._registry.get(self.interface, ())
- components = [component.compmgr[cls] for cls in classes]
+ compmgr = component.compmgr
+ components = [component \
+ if cls not in compmgr and isinstance(component, cls) \
+ else compmgr[cls] \
+ for cls in classes]
return [c for c in components if c]
def __repr__(self):
Thoughts?
--
Jun Omae <jun...@gmail.com> (大前 潤)
Olemis Lang
> Hi,
>
:)
> On Fri, Jan 20, 2012 at 00:44, Olemis Lang <ole...@gmail.com> wrote:
>> I'm writing a patch containing a unit test with a simplified scenario
>> illustrating the barely minimal context for this to happen . The root
>> cause for infinite loop is that after r10295 instances are registered
>> in ComponentManager's «pool» after calling __init__ method (#9418
>> records the whole discussion ;)
>
> I can reproduce the issue and understand the condition. During a
> component is initializing, accessing extension points of the component
> causes infinite loop.
>
yep
> Solution 1.
> Detect and refuse accessing the extension points during initializing
> at `ExtensionPoint.extensions`.
>
[...]
-1 ... as this kind of situations are used by components providing
default implementations for a given interface .
>
> Solution 2.
> When detecting accessing the extension points, use `component` variable
> instead of `component.compmgr[cls]` at `ExtensionPoint.extensions`.
>
> diff --git a/trac/core.py b/trac/core.py
> index 7c03577..13a355d 100644
> --- a/trac/core.py
> +++ b/trac/core.py
> @@ -75,7 +75,11 @@ class ExtensionPoint(property):
> extension point interface.
> """
> classes = ComponentMeta._registry.get(self.interface, ())
> - components = [component.compmgr[cls] for cls in classes]
> + compmgr = component.compmgr
> + components = [component \
> + if cls not in compmgr and isinstance(component, cls) \
> + else compmgr[cls] \
> + for cls in classes]
> return [c for c in components if c]
>
> def __repr__(self):
>
as I already published the patch containing the new test case [1]_ (if
u prefer check out my patch queue @ Bitbucket [2]_ ;)
it'd be nice if u please mention whether it pass or not . If you want
to hack a little in there please let me know in private so as to grant
you with commit access ;)
>
> Thoughts?
>
Solution solution 3 is available ... it allows to do things as usual
by skipping silently nested calls to __init__ .
It comes in the form of two patches
1- adds a test case to detect this [1]_
2- on top of 1 includes solution 3 to the problem i.e. infinite
recursion loop [7]_
Test results :
- Before applying patch 1 : [3]_
- After applying patch 1 [4]_
- After applying patch 2 on top of 1 [5]_ (<= notice that I could not
run the test suite
because I'm experiencing an issue with notification test case [6]_ -Trac
test runner hangs to the point I have to kill -s KILL the process- , if
you have any idea so as to solve this , please fork this conversation
in the ML or meet me @ #trac channel irc.freenode.net ;)
Feedback is welcome
;)
.. [1] Patch 1 : test_default_to_self_extension
(https://bitbucket.org/olemis/trac-mq/src/fa0fcf0f1c3f/tho/themeengine/t-9580-tc-infinite-recursion.diff)
.. [2] Olemis' patch queue for Trac . Used to play & hack & break & ... ;)
(https://bitbucket.org/olemis/trac-mq)
.. [3] Test results before applying patch 1
(http://paste.pocoo.org/show/537773/)
.. [4] Test results after applying patch 1
(http://paste.pocoo.org/show/537774/)
.. [5] Test results after applying patch 2 on top of 1
(http://paste.pocoo.org/show/537776/)
.. [6] Test failure in notification test case while running Trac test suite
(http://paste.pocoo.org/show/537350/)
.. [7] Patch 2 Skipping nested calls to __init__
(https://bitbucket.org/olemis/trac-mq/src/fa0fcf0f1c3f/tho/themeengine/t-9580-infinite-recursion.diff)
--
Regards,
Olemis
Facebook => http://www.facebook.com/olemis
Twitter => http://www.twitter.com/olemislc (@olemislc)
Blog ES => http://simelo-es.blogspot.com
Blog EN => http://simelo-en.blogspot.com
Quora => http://www.quora.com/olemis
Youtube => http://youtube.com/user/greatsoftw
Featured article : Identificando números primos con expresión regular en Perl
http://feedproxy.google.com/~r/simelo-news/~3/BHr859OSndo/identificando-numeros-primos-con.html
Tweet: omg ! seems I finished d first version of d patch 4
http://t.co/ZWRrJaW7 http://t.co/cE3Q6Yfb . thnx @oddsimons #trac
Follow @olemislc Reply Retweet 17:24 Jan-19
Christian Boos
> On Fri, Jan 20, 2012 at 2:58 AM, Jun Omae<jun...@gmail.com> wrote:
>> Hi,
>>
>
> :)
>
>> On Fri, Jan 20, 2012 at 00:44, Olemis Lang<ole...@gmail.com> wrote:
>>> I'm writing a patch containing a unit test with a simplified scenario
>>> illustrating the barely minimal context for this to happen . The root
>>> cause for infinite loop is that after r10295 instances are registered
>>> in ComponentManager's �pool� after calling __init__ method (#9418
>>> records the whole discussion ;)
>>
>> I can reproduce the issue and understand the condition. During a
>> component is initializing, accessing extension points of the component
>> causes infinite loop.
>>
>
> yep
And this is an error. 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.
This is documented:
http://www.edgewall.org/docs/trac-trunk/html/api/trac_core.html#more-on-components
Well, I could add a more specific note about the temptation to
access the extension points from there, as apparently it's not
implicit.
>
>> Solution 1.
>> Detect and refuse accessing the extension points during initializing
>> at `ExtensionPoint.extensions`.
>>
> [...]
Either this, or leave things as they are.
>
> -1 ... as this kind of situations are used by components providing
> default implementations for a given interface .
Simply set some flag and do the initialization *later* when
needed (well, you should also possibly need a lock but I won't
get into this now - simply look at the various examples in the
Trac code itself).
>
>>
>> Solution 2.
>> When detecting accessing the extension points, use `component` variable
>> instead of `component.compmgr[cls]` at `ExtensionPoint.extensions`.
>>
No, this would present a misleading list, and moreover give the
impression that it's OK to do such things.
>
> Solution solution 3 is available ... it allows to do things as usual
> by skipping silently nested calls to __init__ .
> It comes in the form of two patches
>
> 1- adds a test case to detect this [1]_
> 2- on top of 1 includes solution 3 to the problem i.e. infinite
> recursion loop [7]_
>
A bit too complex for this corner case, no? Perhaps if that would
have been a legit corner case, but it's not.
-- Christian
Olemis Lang
+1
;)
>>
>>> Solution 1.
>>> Detect and refuse accessing the extension points during initializing
>>> at `ExtensionPoint.extensions`.
>>>
>> [...]
>
> Either this, or leave things as they are.
>
Considering what's been said above I'd rather go for the former .
Better explicit than implicit
;)
>
>>
>>> Solution 2.
>>> When detecting accessing the extension points, use `component` variable
>>> instead of `component.compmgr[cls]` at `ExtensionPoint.extensions`.
>>>
>
> No, this would present a misleading list, and moreover give the
> impression that it's OK to do such things.
>
I agree
-1
>
>> Solution solution 3 is available ... it allows to do things as usual
>> by skipping silently nested calls to __init__ .
>> It comes in the form of two patches
>>
>> 1- adds a test case to detect this [1]_
>> 2- on top of 1 includes solution 3 to the problem i.e. infinite
>> recursion loop [7]_
>
> A bit too complex for this corner case, no? Perhaps if that would
> have been a legit corner case, but it's not.
>
ok . thnx for the replies .
:)
--
Regards,
Olemis
Facebook => http://www.facebook.com/olemis
Twitter => http://www.twitter.com/olemislc (@olemislc)
Blog ES => http://simelo-es.blogspot.com
Blog EN => http://simelo-en.blogspot.com
Quora => http://www.quora.com/olemis
Youtube => http://youtube.com/user/greatsoftw
Featured article : Identificando números primos con expresión regular en Perl
http://feedproxy.google.com/~r/simelo-news/~3/BHr859OSndo/identificando-numeros-primos-con.html
Tweet: RT @WANdisco How you can add #uTest to #uberSVN...
http://t.co/SCUhNd6B #fb
Follow @olemislc Reply Retweet 12:35 Jan-20