"Recursion depth exceeded" while scrolling #89

Closed
Blaisorblade opened this Issue Sep 7, 2011 · 5 comments

Comments

Projects
None yet
2 participants
@Blaisorblade

I noticed the following stacktrace while scrolling - the actual trace was longer and did not fit the console history (whose depth is 1000 lines), but looked perfectly periodic, as visibile below. I observed the issue while changing diff viewer's font size through Apple+mouse wheel (on a Mac - that would be equivalent to Ctrl+mouse wheel on non-Mac machines, I assume). The option window was also open at some point. I managed to reproduce a similar trace by playing enough with the option window.

File "/usr/local/share/git-cola/lib/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/usr/local/share/git-cola/lib/cola/models/observable.py", line 20, in set_param
self.notify_observers(param)
File "/usr/local/share/git-cola/lib/cola/observable.py", line 36, in notify_observers
observer.notify(_param)
File "/usr/local/share/git-cola/lib/cola/observer.py", line 24, in notify
self.subject_changed(attr, value)
File "/usr/local/share/git-cola/lib/cola/qobserver.py", line 290, in subject_changed
action(_widgets)
File "/usr/local/share/git-cola/lib/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/usr/local/share/git-cola/lib/cola/models/observable.py", line 20, in set_param
self.notify_observers(param)
File "/usr/local/share/git-cola/lib/cola/observable.py", line 36, in notify_observers
observer.notify(_param)
File "/usr/local/share/git-cola/lib/cola/observer.py", line 24, in notify
self.subject_changed(attr, value)
File "/usr/local/share/git-cola/lib/cola/qobserver.py", line 290, in subject_changed
action(_widgets)
File "/usr/local/share/git-cola/lib/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/usr/local/share/git-cola/lib/cola/models/observable.py", line 20, in set_param
self.notify_observers(param)
File "/usr/local/share/git-cola/lib/cola/observable.py", line 36, in notify_observers
observer.notify(_param)
File "/usr/local/share/git-cola/lib/cola/observer.py", line 24, in notify
self.subject_changed(attr, value)
File "/usr/local/share/git-cola/lib/cola/qobserver.py", line 290, in subject_changed
action(_widgets)
File "/usr/local/share/git-cola/lib/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/usr/local/share/git-cola/lib/cola/models/observable.py", line 20, in set_param
self.notify_observers(param)
File "/usr/local/share/git-cola/lib/cola/observable.py", line 36, in notify_observers
observer.notify(_param)
File "/usr/local/share/git-cola/lib/cola/observer.py", line 24, in notify
self.subject_changed(attr, value)
File "/usr/local/share/git-cola/lib/cola/qobserver.py", line 290, in subject_changed
action(_widgets)
File "/usr/local/share/git-cola/lib/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/usr/local/share/git-cola/lib/cola/decorators.py", line 18, in _decorator
return caller(func, _args, *_opts)
RuntimeError: maximum recursion depth exceeded

Here's the second stack trace:
File "/usr/local/share/git-cola/lib/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/usr/local/share/git-cola/lib/cola/models/observable.py", line 20, in set_param
self.notify_observers(param)
File "/usr/local/share/git-cola/lib/cola/observable.py", line 36, in notify_observers
observer.notify(_param)
File "/usr/local/share/git-cola/lib/cola/observer.py", line 24, in notify
self.subject_changed(attr, value)
File "/usr/local/share/git-cola/lib/cola/qobserver.py", line 290, in subject_changed
action(_widgets)
File "/usr/local/share/git-cola/lib/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/usr/local/share/git-cola/lib/cola/models/observable.py", line 17, in set_param
Model.set_param(self, param, value)
RuntimeError: maximum recursion depth exceeded while calling a Python object

To obtain this, I tried something along this lines:

  • Change font size through Apple+mouse wheel
  • Open Options window through "Apple+," (or Apple -> Preferences).
  • Change the font size through the window
    Sometimes, but not needed - Change the font size also through the mouse wheel on the diff viewer
  • Save the changes
  • Change again the font size through Apple+mouse wheel
  • Reopen the Options window - now the stored value is probably not consistent with the actual one
  • Change the font size in the option window - in some cases, the diff viewer will not be updated (the number of iterations needed varies between different attempts).
  • If the diff viewer is not updated, clicking "save" will cause one of the above stack traces.

@davvid davvid closed this Sep 7, 2011

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Sep 7, 2011

Member

Thanks for reporting this issue. I pushed a branch called "recursion-fix" that may address this bug. Can you try using that branch and see if it indeed resolved this issue? If so I'll go ahead and merge it into master.

You can run git-cola from it's source tree (against another project) by doing something along these lines:

cd src
git clone git://github.com/davvid/git-cola.git
cd git-cola
git checkout recursive-fix
cd ../some_other_project
../git-cola/bin/git-cola

Basically, just call the git-cola script directly and you will be running a development version.

Member

davvid commented Sep 7, 2011

Thanks for reporting this issue. I pushed a branch called "recursion-fix" that may address this bug. Can you try using that branch and see if it indeed resolved this issue? If so I'll go ahead and merge it into master.

You can run git-cola from it's source tree (against another project) by doing something along these lines:

cd src
git clone git://github.com/davvid/git-cola.git
cd git-cola
git checkout recursive-fix
cd ../some_other_project
../git-cola/bin/git-cola

Basically, just call the git-cola script directly and you will be running a development version.

@Blaisorblade

This comment has been minimized.

Show comment
Hide comment
@Blaisorblade

Blaisorblade Sep 8, 2011

Thanks for the prompt response. However, I can still reproduce the problem - see stacktrace below (line numbers changed slightly). Note that the bug is not always reproduceable at first attempt, one must repeat the procedure a variable number of times. I had also forgot to say that I first always select a file with changes, so that the diff viewer has something to show.
Moreover, I just discovered that the stack trace is thrown not when saving the preferences, but when adjusting the font size - exactly the first time that I change the font size in the window pane and the font size in the preview window is not changed.
Moreover, when adjusting the font size through scrolling, the value in preferences is never adjusted - is that intentional?

Finally, can I also request an option to disable Apple-scroll? I always do that unintentionally. If you want me to open another tracker item, as I guess I should, I can do that.

File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/models/observable.py", line 20, in set_param
self.notify_observers(param)
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/observable.py", line 35, in notify_observers
observer.notify(_param)
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/observer.py", line 24, in notify
self.subject_changed(attr, value)
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/qobserver.py", line 303, in subject_changed
action(_widgets)
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/decorators.py", line 18, in _decorator
return caller(func, _args, *_opts)
RuntimeError: maximum recursion depth exceeded

Thanks for the prompt response. However, I can still reproduce the problem - see stacktrace below (line numbers changed slightly). Note that the bug is not always reproduceable at first attempt, one must repeat the procedure a variable number of times. I had also forgot to say that I first always select a file with changes, so that the diff viewer has something to show.
Moreover, I just discovered that the stack trace is thrown not when saving the preferences, but when adjusting the font size - exactly the first time that I change the font size in the window pane and the font size in the preview window is not changed.
Moreover, when adjusting the font size through scrolling, the value in preferences is never adjusted - is that intentional?

Finally, can I also request an option to disable Apple-scroll? I always do that unintentionally. If you want me to open another tracker item, as I guess I should, I can do that.

File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/models/observable.py", line 20, in set_param
self.notify_observers(param)
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/observable.py", line 35, in notify_observers
observer.notify(_param)
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/observer.py", line 24, in notify
self.subject_changed(attr, value)
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/qobserver.py", line 303, in subject_changed
action(_widgets)
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/controllers/options.py", line 105, in tell_parent_model
cola.model().set_param(param, self.model.param(param))
File "/Users/pgiarrusso/Documents/Admin/git-cola/cola/decorators.py", line 18, in _decorator
return caller(func, _args, *_opts)
RuntimeError: maximum recursion depth exceeded

@davvid davvid reopened this Sep 10, 2011

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Sep 10, 2011

Member

That whole part of the code is something I've been meaning to rewrite. I wrote it a long time ago when I was a bit newer to Qt and much more magically inclined. I haven't repro'd this yet but I've been on linux this whole time. I wouldn't expect that to matter. I'll try this on the mac soon too, but it'd almost be better to rewrite the options dialog to use editor commands and the notification stuff instead of QObserver. this bug should really be called "War on QObserver" or something. anyways, reopening.

Member

davvid commented Sep 10, 2011

That whole part of the code is something I've been meaning to rewrite. I wrote it a long time ago when I was a bit newer to Qt and much more magically inclined. I haven't repro'd this yet but I've been on linux this whole time. I wouldn't expect that to matter. I'll try this on the mac soon too, but it'd almost be better to rewrite the options dialog to use editor commands and the notification stuff instead of QObserver. this bug should really be called "War on QObserver" or something. anyways, reopening.

@davvid davvid closed this in 573f0e6 Sep 13, 2011

@Blaisorblade

This comment has been minimized.

Show comment
Hide comment
@Blaisorblade

Blaisorblade Sep 15, 2011

Thanks for the fix, seems to work for now (I'm testing v1.4.3.5-89-gb0589e1)!
Only a minor thing: May I suggest to use a tabbed pane instead of a combo box? It really bothers me to need two clicks to switch the option pane. Thanks!

Thanks for the fix, seems to work for now (I'm testing v1.4.3.5-89-gb0589e1)!
Only a minor thing: May I suggest to use a tabbed pane instead of a combo box? It really bothers me to need two clicks to switch the option pane. Thanks!

@davvid davvid reopened this Sep 16, 2011

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Sep 16, 2011

Member

Great suggestion!

Member

davvid commented Sep 16, 2011

Great suggestion!

@davvid davvid closed this in 177c116 Sep 16, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment