Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent inspector from adding itself to the window #5658

Closed
wants to merge 22 commits into from

Conversation

Zen-CODE
Copy link
Member

@Zen-CODE Zen-CODE commented Mar 17, 2018

Based on #5645, I managed to reproduce
the Inspector "already has a parent" error but it happens immediately I pres "Ctrl + e" to open
the inspector and only when a popup has been created.
To debug this, I monitored the 'on_parent' event of the inspector. It turns
our the Inspector binds to the 'children' event of the window and then tries
to add itself when whenever th 'children' Window event` fires.

As the ModalView adds itself directly to the Window, the event fires and the
Inspector adds itself even when it has not been activated. This is clearly
not desired behavior.

THe PR adds a check to the Inpector to detect that it does indeed want to
re-insert itself into the children.

@bionid. Please confirm it fixes #5645 as I
could only reproduce this when creating the popup, not on closing the
inspector as your ticket indicates. It also seems to fix the error in the travis builds, so those should pass if this is merged :-)

@ghost
Copy link

ghost commented Mar 18, 2018

Yup, confirmed, this works!

I guess you should apply it to console and joycursor modules as well

@Zen-CODE
Copy link
Member Author

Zen-CODE commented Mar 18, 2018 via email

@ghost
Copy link

ghost commented Mar 18, 2018

I just noticed those modules appeared to do the same thing

they don't have an 'activated' property, so this fix is not directly applicable.

Am I missing something here?

@Zen-CODE
Copy link
Member Author

Zen-CODE commented Mar 20, 2018

Indeed not. Not sure how I missed those when searching, but nice catch. Thanks.

So, when using the console, it appears immediately when the popup does. These changes fix that.

When using the joycursor, simply deactivating it throws:

 Traceback (most recent call last):
   File "kivy/properties.pyx", line 571, in kivy.properties.Property.dispatch
   File "kivy/_event.pyx", line 1214, in kivy._event.EventObservers.dispatch
   File "kivy/_event.pyx", line 1120, in kivy._event.EventObservers._dispatch
   File "/usr/local/lib/python2.7/dist-packages/kivy/modules/joycursor.py", line 127, in on_window_children
     win.add_widget(self)
   File "/usr/local/lib/python2.7/dist-packages/kivy/core/window/__init__.py", line 1247, in add_widget
     (widget, widget.parent)
 kivy.uix.widget.WidgetException: Cannot add <kivy.modules.joycursor.JoyCursor object at 0x7f5838e13ec0> to window, it already has a parent <kivy.core.window.window_pygame.WindowPygame object at 0x7f583c1c9670>
 Exception kivy.uix.widget.WidgetException: WidgetException('Cannot add <kivy.modules.joycursor.JoyCursor object at 0x7f5838e13ec0> to window, it already has a parent <kivy.core.window.window_pygame.WindowPygame object at 0x7f583c1c9670>',) in 'kivy.properties.observable_list_dispatch' ignored
[INFO   ] [JoyCursor   ] joycursor deactivated

These change fix that too.

@Zen-CODE Zen-CODE changed the title Prevent inspector from adding itself to the window [WIP] Prevent inspector from adding itself to the window Mar 21, 2018
@Zen-CODE
Copy link
Member Author

Hmm. There is an annoying issue with the travis builds where it seems to hang after the graphical unit tests. Just doing some commits to experiment and see if I can find the issue....

@akshayaurora
Copy link
Member

@Zen-CODE is this still WIP ?

@Zen-CODE Zen-CODE changed the title [WIP] Prevent inspector from adding itself to the window Prevent inspector from adding itself to the window Mar 28, 2018
@Zen-CODE
Copy link
Member Author

I think it's ready to be merged. It does fix the issues mentioned above.

The reason I was holding it back is that I hit an issue in the travis builds. In earlier commits, it passed all travis tests. Then suddenly, it start hanging on the last 'test_widget_popup' test in kivy/tests/test_module_inspector.py'. It works fine on my machine, but when push to github, it does not get past the last 'self.render(self.root)' call on line 296. The test then times out.

But since, I've tested with and without the changes. Both never get past that line. So these fixes do not introduce this issue at least. But it's strange that they did pass initially....

@ghost
Copy link

ghost commented Apr 14, 2018

@Zen-CODE According to travis-ci/travis-ci#6861 the hang may be caused by xvfb background process. It may help to do /sbin/start-stop-daemon --stop in after_script

@Zen-CODE
Copy link
Member Author

@bionid. Is that what you meant?
11e9307

Now we seem to get a a new set of errors...

@ghost
Copy link

ghost commented Apr 16, 2018

@Zen-CODE well it needs to refer to the xvfb process that is started here. I'm not sure the exact syntax to stop it correctly, so I left out the rest of the arguments as an exercise for the reader ;-)

As a test (or solution..) you could try

kill -9 `cat /tmp/custom_xvfb_99.pid`

@Zen-CODE
Copy link
Member Author

This seems like a hack route. I wish there were some way of finding why it does not exit cleanly on the travis builds but is fine on a running locally. It's probably related to the need for this PR. Something in that last 'test_widget_popup' test is different but I cannot see what....

The mocks around the app event loop make it difficult to debug or follow. Not sure what to do from here. It solves the original errors, so is maybe worth merging? Or how to identify to reason for the hang?

@ghost
Copy link

ghost commented Apr 21, 2018

@Zen-CODE I have no idea about the details concerning travis, but the fix looks good & resolves the issue, from my perspective good to merge... But this pr is 19 commits for +8-3 lines, better to squash before merge

@Zen-CODE
Copy link
Member Author

Zen-CODE commented Apr 21, 2018 via email

@Zen-CODE Zen-CODE closed this Apr 21, 2018
@Zen-CODE
Copy link
Member Author

#5705

@Zen-CODE Zen-CODE deleted the inspector_fix branch April 21, 2018 20:49
tshirtman added a commit that referenced this pull request May 12, 2018
the root cause of the issue hasn't been identified but:

- the render() call blocks, apparently waiting for a window flip event
- it's been introduced by commit be1ce, which prevents adding a widget
  to a window if it already has a parent.
- checking in inspector if it already has a parent before adding it to
  window doesn't seem to fix the issue.

see #5645 ##5646 #5658
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Closing inspector throws exception after merging #5612
2 participants