wxWidgets / Phoenix

wxPython's Project Phoenix. A new implementation of wxPython, better, stronger, faster than he was before.
http://wxpython.org/
2.23k stars 510 forks source link

Application crash using AGW AUI #1881

Open kdschlosser opened 3 years ago

kdschlosser commented 3 years ago

Operating system: Windows 7 x64 wxPython version & source: wxPython 4.1.1 (pypi) Python version & source: CPython 3.8.6 (python.org)

Description of the problem: When docking a pane I get an application crash. 0xC0000005: Access violation

The crash occurs in wxbase315u_vc140_x64.dll Line 1637 of wxWidgets/src/common/event.cpp return ProcessEvent(event); in function wxEvtHandler::SafelyProcessEvent(wxEvent & event)

the wx.Frame has been constructed with these flags wx.MINIMIZE_BOX wx.MAXIMIZE_BOX wx.RESIZE_BORDER wx.CAPTION wx.SYSTEM_MENU wx.CLOSE_BOX wx.CLIP_CHILDREN wx.TAB_TRAVERSAL

the wx.lib.agw.aui.AuiManager has been constructed with these flags aui.AUI_MGR_ALLOW_FLOATING aui.AUI_MGR_ALLOW_ACTIVE_PANE aui.AUI_MGR_TRANSPARENT_DRAG aui.AUI_MGR_VENETIAN_BLINDS_HINT aui.AUI_MGR_HINT_FADE aui.AUI_MGR_LIVE_RESIZE aui.AUI_MGR_ANIMATE_FRAMES aui.AUI_MGR_WHIDBEY_DOCKING_GUIDES aui.AUI_MGR_PREVIEW_MINIMIZED_PANES aui.AUI_MGR_SMOOTH_DOCKING

the wx.lib.agw.aui.AuiPaneInfo as been constructed using these options Name(name) Caption(caption) Row(0) MinSize((300, 300)) FloatingSize((300, 300)) MinimizeMode(aui.AUI_MINIMIZE_POS_BOTTOM | aui.AUI_MINIMIZE_CAPT_SMART) Resizable(True) Show() CaptionVisible(True, left=True) PaneBorder(True) Gripper(True) GripperTop(True) CloseButton(True) MaximizeButton(True) MinimizeButton(True) PinButton(True) DestroyOnClose(True) Floatable(True) Movable(True) Dockable(True) Snappable(True) FlyOut(True)

the "window" the pane is being populated with is a wx.ListCtrl instance

What I am doing is I am clicking on the gripper of the pane and moving it, once I move it the docking guides appear. I move the mouse over the right arrow in the docking guide. It then shows where the pane will be docked, I then let go of the mouse button the pane gets docked and then application crash.

I am not binding to any AUI events so I know I am not inadvertently stepping on somethings toes.

I am sub classing wx.lib.agw.aui.framemanager.AuiPaneInfo only for the purposes of setting up the pane and the only method I have overridden is __init__. Could this cause the problem?

I have attached the Visual Studio debug dump file. agw_aui_crash_dump.zip

Thanks for the help

kdschlosser commented 3 years ago

I just removed the subclassing of the AuiPaneInfo and it still crashes.

kdschlosser commented 3 years ago

If I float the pane first and then dock it the crash still occurs. I am going to code up the most basic example to see if it still does it.

kdschlosser commented 3 years ago

Ok I did up a test that only has the necessities. It still crashes.

Here is the code

import wx
from wx.lib.agw import aui

app = wx.App()

class MainFrame(wx.Frame):

    def __init__(self, parent=None):
        wx.Frame.__init__(
            self,
            parent,
            -1,
            size=(600, 600),
            pos=(0, 0)
        )

        self.manager = aui.AuiManager(self)

        center_panel = wx.Panel(self, -1, size=(300, 300), style=wx.BORDER_NONE)

        pane = PaneInfo(center_panel, 'center_pane', 'Center Pane')
        pane.CenterPane()
        pane.add_to_manager()

        panel = wx.Panel(self, -1, size=(300, 300), style=wx.BORDER_NONE)

        pane = PaneInfo(panel, 'test_pan', 'Test Pane')
        pane.add_to_manager()

def PaneInfo(window, name, caption):
    pane = aui.AuiPaneInfo()
    pane.Name(name)
    pane.Caption(caption)
    pane.Row(0)
    pane.MinSize((300, 300))
    pane.FloatingSize((300, 300))
    pane.MinimizeMode(aui.AUI_MINIMIZE_POS_BOTTOM | aui.AUI_MINIMIZE_CAPT_SMART)
    pane.Resizable(True)
    pane.Show()
    pane.CaptionVisible(True, left=True)
    pane.PaneBorder(True)
    pane.Gripper(True)
    pane.GripperTop(True)
    pane.CloseButton(True)
    pane.MaximizeButton(True)
    pane.MinimizeButton(True)
    pane.PinButton(True)
    pane.DestroyOnClose(True)
    pane.Floatable(True)
    pane.Movable(True)
    pane.Dockable(True)
    pane.Snappable(True)
    pane.FlyOut(True)

    class AddToManager(object):

        def __init__(self, w, p):
            self.window = w
            self.pane = p

        def __call__(self):
            def _get_main_frame(child):
                parent = child.GetParent()
                if parent is None:
                    return child.manager

                return _get_main_frame(parent)

            manager = _get_main_frame(self.window)

            manager.AddPane(self.window, self.pane)
            manager.ShowPane(self.window, True)
            manager.Update()

    pane.add_to_manager = AddToManager(window, pane)

    return pane

frame = MainFrame()
frame.Show()

app.MainLoop()
kdschlosser commented 3 years ago

OK I tracked down the problem to the FlyOut option for the AuiPaneInfo.

I do not know exactly what about the option is causing the problem But if I omit it the problem goes away.

infinity77 commented 3 years ago

Hi,

I haven’t tried your code so I can’t comment on the crash, but I’d like

to say that it is a fantastically convoluted and needlessly complex way to use AUI. Is there any reason why it needs to be done that way?

On Tue, 22 Dec 2020 at 07.31, Kevin Schlosser notifications@github.com wrote:

Ok I did up a test that only has the necessities. It still crashes.

Here is the code

import wxfrom wx.lib.agw import aui app = wx.App() class MainFrame(wx.Frame):

def __init__(self, parent=None):
    wx.Frame.__init__(
        self,
        parent,
        -1,
        size=(600, 600),
        pos=(0, 0),
        style=(
            wx.MINIMIZE_BOX |
            wx.MAXIMIZE_BOX |
            wx.RESIZE_BORDER |
            wx.CAPTION |
            wx.SYSTEM_MENU |
            wx.CLOSE_BOX |
            wx.CLIP_CHILDREN |
            wx.TAB_TRAVERSAL
        )
    )

    self.manager = aui.AuiManager(
        self,
        agwFlags=(
            aui.AUI_MGR_ALLOW_FLOATING |
            aui.AUI_MGR_ALLOW_ACTIVE_PANE |
            aui.AUI_MGR_TRANSPARENT_DRAG |
            # AUI_MGR_TRANSPARENT_HINT |
            aui.AUI_MGR_VENETIAN_BLINDS_HINT |
            # AUI_MGR_RECTANGLE_HINT
            aui.AUI_MGR_HINT_FADE |
            # AUI_MGR_NO_VENETIAN_BLINDS_FADE
            aui.AUI_MGR_LIVE_RESIZE |
            aui.AUI_MGR_ANIMATE_FRAMES |
            # AUI_MGR_AERO_DOCKING_GUIDES
            aui.AUI_MGR_WHIDBEY_DOCKING_GUIDES |
            aui.AUI_MGR_PREVIEW_MINIMIZED_PANES |
            aui.AUI_MGR_SMOOTH_DOCKING
        )
    )

    center_panel = wx.Panel(self, -1, size=(300, 300), style=wx.BORDER_NONE)

    pane = PaneInfo(center_panel, 'center_pane', 'Center Pane')
    pane.CenterPane()
    pane.add_to_manager()

    panel = wx.Panel(self, -1, size=(300, 300), style=wx.BORDER_NONE)

    pane = PaneInfo(panel, 'test_pan', 'Test Pane')
    pane.add_to_manager()

def PaneInfo(window, name, caption): pane = aui.AuiPaneInfo() pane.Name(name) pane.Caption(caption) pane.Row(0) pane.MinSize((300, 300)) pane.FloatingSize((300, 300)) pane.MinimizeMode(aui.AUI_MINIMIZE_POS_BOTTOM | aui.AUI_MINIMIZE_CAPT_SMART) pane.Resizable(True) pane.Show() pane.CaptionVisible(True, left=True) pane.PaneBorder(True) pane.Gripper(True) pane.GripperTop(True) pane.CloseButton(True) pane.MaximizeButton(True) pane.MinimizeButton(True) pane.PinButton(True) pane.DestroyOnClose(True) pane.Floatable(True) pane.Movable(True) pane.Dockable(True) pane.Snappable(True) pane.FlyOut(True)

class AddToManager(object):

    def __init__(self, w, p):
        self.window = w
        self.pane = p

    def __call__(self):
        def _get_main_frame(child):
            parent = child.GetParent()
            if parent is None:
                return child.manager

            return _get_main_frame(parent)

        manager = _get_main_frame(self.window)

        manager.AddPane(self.window, self.pane)
        manager.ShowPane(self.window, True)
        manager.Update()

pane.add_to_manager = AddToManager(window, pane)

return pane

frame = MainFrame()frame.Show() app.MainLoop()

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/wxWidgets/Phoenix/issues/1881#issuecomment-749370439, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACESNIOPJV2QKWPBPFGICKLSWA4NXANCNFSM4VFFWFXA .

kdschlosser commented 3 years ago

complex? There is an easier way?

infinity77 commented 3 years ago

Yes, one of the simplest is the main example in the documentation:

https://wxpython.org/Phoenix/docs/html/wx.lib.agw.aui.html

On Tue, 22 Dec 2020 at 07.42, Kevin Schlosser notifications@github.com wrote:

complex? There is an easier way?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/wxWidgets/Phoenix/issues/1881#issuecomment-749375235, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACESNINFIG6MAGJ5BFQLI4DSWA5T7ANCNFSM4VFFWFXA .

kdschlosser commented 3 years ago

so what you are saying is that if a user does something inside of a pane that causes a new pane to be created I would have to pass the instance of the top level window (Frame) all the way to the class that is creating the pane so it can call a function in the manager that would then add the pane?

and in my program there are 20 different types of panes that can be made. so there would need to be 20 lines like what you see below in order to create a pane?

self._mgr.AddPane(text1, aui.AuiPaneInfo().Name(name).Caption(caption).Row(0).MinSize((300, 300)).FloatingSize((300, 300)).MinimizeMode(aui.AUI_MINIMIZE_POS_BOTTOM | aui.AUI_MINIMIZE_CAPT_SMART).Resizable(True).Show().CaptionVisible(True, left=True).PaneBorder(True).Gripper(True).GripperTop(True).CloseButton(True).MaximizeButton(True).MinimizeButton(True).PinButton(True).DestroyOnClose(True).Floatable(True).Movable(True).Dockable(True).Snappable(True))

Sorry but I am going to have to disagree with you on my way being more complex. My way if I need to create a pane all I need do is call that function passing 3 parameters to it. I can then modify any pane settings that may need to be modified and call a function that is added to the AuiPaneInfo instance to add it to the AuiManager instance. I do not need to pass the "main frame" or the manager instance to every object that is made in the program in order to add a pane. I also do not have a heap of duplicate code.

kdschlosser commented 3 years ago

Also the function for only used for the purposes of testing. This is the code I used in place of the function

from wx.lib.agw import aui

class PaneInfo(aui.AuiPaneInfo):
    def __init__(self, window, name, caption):
        aui.AuiPaneInfo.__init__(self)
        self.__window = window

        self.Name(name)
        self.Caption(caption)
        self.Row(0)
        self.MinSize((300, 300))
        self.FloatingSize((300, 300))
        self.MinimizeMode(aui.AUI_MINIMIZE_POS_BOTTOM | aui.AUI_MINIMIZE_CAPT_SMART)
        self.Resizable(True)
        self.Show()
        self.CaptionVisible(True, left=True)
        self.PaneBorder(True)
        self.Gripper(True)
        self.GripperTop(True)
        self.CloseButton(True)
        self.MaximizeButton(True)
        self.MinimizeButton(True)
        self.PinButton(True)
        self.DestroyOnClose(True)
        self.Floatable(True)
        self.Movable(True)
        self.Dockable(True)
        self.Snappable(True)
        # pane.FlyOut(True)

    def add_to_manager(self):
        def _get_main_frame(child):
            parent = child.GetParent()
            if parent is None:
                return child.manager

            return _get_main_frame(parent)

        manager = _get_main_frame(self.__window)

        manager.AddPane(self.__window, self)
        manager.ShowPane(self.__window, True)
        manager.Update()
infinity77 commented 3 years ago

You wouldn’t have any heap of duplicates code either if you did that in a loop, instead of having a subclassed AuiPaneInfo which contains a special method which in turn contains another class... but I guess it’s up to personal preferences.

That said, I’m not aware of many people using the FlyOut option of AUI. What I’m sure about is that it used to work back in time in wxPython Classic. Does it work in the demo? If not, then something new in Phoenix must have changed.

On Tue, 22 Dec 2020 at 08.05, Kevin Schlosser notifications@github.com wrote:

so what you are saying is that if a user does something inside of a pane that causes a new pane to be created I would have to pass the instance of the top level window (Frame) all the way to the class that is creating the pane so it can call a function in the manager that would then add the pane?

and in my program there are 20 different types of panes that can be made. so there would need to be 20 lines like what you see below in order to create a pane?

self._mgr.AddPane(text1, aui.AuiPaneInfo().Name(name).Caption(caption).Row(0).MinSize((300, 300)).FloatingSize((300, 300)).MinimizeMode(aui.AUI_MINIMIZE_POS_BOTTOM | aui.AUI_MINIMIZE_CAPT_SMART).Resizable(True).Show().CaptionVisible(True, left=True).PaneBorder(True).Gripper(True).GripperTop(True).CloseButton(True).MaximizeButton(True).MinimizeButton(True).PinButton(True).DestroyOnClose(True).Floatable(True).Movable(True).Dockable(True).Snappable(True))

Sorry but I am going to have to disagree with you on my way being more complex. My way if I need to create a pane all I need do is call that function passing 3 parameters to it. I can then modify any pane settings that may need to be modified and call a function that is added to the AuiPaneInfo instance to add it to the AuiManager instance. I do not need to pass the "main frame" or the manager instance to every object that is made in the program in order to add a pane. I also do not have a heap of duplicate code.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/wxWidgets/Phoenix/issues/1881#issuecomment-749384502, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACESNIMOZR32QVXXJAB4PLLSWBANBANCNFSM4VFFWFXA .

kdschlosser commented 3 years ago

I do not even know what the Flyout option is supposed to do. I basically looked at the AUIPaneInfo class grabbed the settings and added them. When I go through testing I will see what is what.

There is another error when I minimize the pane and I hover over the minimized pane. I should get a preview window. This is not working.

Traceback (most recent call last):
  File "C:\Python38\lib\site-packages\wx\lib\agw\aui\auibar.py", line 3929, in OnMotion
    self.StopPreviewTimer()
  File "C:\Python38\lib\site-packages\wx\lib\agw\aui\auibar.py", line 4030, in StopPreviewTimer
    manager.StopPreviewTimer()
  File "C:\Python38\lib\site-packages\wx\lib\agw\aui\framemanager.py", line 10621, in StopPreviewTimer
    self.SlideOut()
  File "C:\Python38\lib\site-packages\wx\lib\agw\aui\framemanager.py", line 10680, in SlideOut
    for i in range(stopX, 0, -step):
TypeError: 'float' object cannot be interpreted as an integer

All of the AUI code is going to have to be gone over to make it Python 3 compatible. step = stopX/10 returns a float and range does not accept floats, step = stopX // 10 will fix the issue.

I suspect there are going to be quite a few odd things happening in AUI because of not using floor division in Python 3

The code in RescaleScreenShot can also be optimized for Python 3. While there is no technical error in it it is not going to be the most efficient code.

here is an example

import timeit

data = """\
x = 20000
y = float(x) / 2.0
y = int(y/2.0)
"""

res = timeit.timeit(stmt=data, number=100000)

print(res)

data = """\
x = 20000
y = x / 2
y = y // 2
"""

res = timeit.timeit(stmt=data, number=100000)

print(res)

the above outputs

0.06544675
0.035539882999999994

the use of int() and float() instead of using the floor division operator and letting Python do what it does with division operator causes the code execution time to take close to 2 times longer.

I know it was done that way because of Python 2, But it needs to be updated so it can run at optimal performance in Python 3.

The above is only a single example of the kind of math found in a single function in AUI. I am sure that because of Python 2 there is more math done like this.

infinity77 commented 3 years ago

Hi,

It’s definitely probable that there are quite a few things to be

checked in order to define AUI (and other modules) as 100% fully ported to Python 3. I don’t think anyone has tested every single possible option of AUI, for example, and I guess the AUI unit-tests are not as comprehensive as needed. That said, I’m sure Robin will accept a patch on that issue - and any other issue you might discover.

The first version of the pure-Python AUI dates back to when Python 3 was still in its unfortunate planning process, so there’s plenty to be done on AUI.

Optimization-wise, I’m also quite positive that major gains could be had by refactoring the code, eliminating or tightening loops, and simply do things in the Python way instead of the C++ way. I’m a bit more dubious about micro-changes on int/float operations and/or attribute access optimizations, as a 300 nanoseconds savings is unlikely to make a visible contribution. That said, every improvement counts, and I’m positive the community will be grateful for PRs addressing any issue/performance bottleneck.

Andrea.

On Tue, 22 Dec 2020 at 08.49, Kevin Schlosser notifications@github.com wrote:

I do not even know what the Flyout option is supposed to do. I basically looked at the AUIPaneInfo class grabbed the settings and added them. When I go through testing I will see what is what.

There is another error when I minimize the pane and I hover over the minimized pane. I should get a preview window. This is not working.

Traceback (most recent call last): File "C:\Python38\lib\site-packages\wx\lib\agw\aui\auibar.py", line 3929, in OnMotion self.StopPreviewTimer() File "C:\Python38\lib\site-packages\wx\lib\agw\aui\auibar.py", line 4030, in StopPreviewTimer manager.StopPreviewTimer() File "C:\Python38\lib\site-packages\wx\lib\agw\aui\framemanager.py", line 10621, in StopPreviewTimer self.SlideOut() File "C:\Python38\lib\site-packages\wx\lib\agw\aui\framemanager.py", line 10680, in SlideOut for i in range(stopX, 0, -step): TypeError: 'float' object cannot be interpreted as an integer

All of the AUI code is going to have to be gone over to make it Python 3 compatible. step = stopX/10 returns a float and range does not accept floats, step = stopX // 10 will fix the issue.

I suspect there are going to be quite a few odd things happening in AUI because of not using floor division in Python 3

The code in RescaleScreenShot can also be optimized for Python 3. While there is no technical error in it it is not going to be the most efficient code.

here is an example

import timeit data = """\x = 20000y = float(x) / 2.0y = int(y/2.0)""" res = timeit.timeit(stmt=data, number=100000) print(res) data = """\x = 20000y = x / 2y = y // 2""" res = timeit.timeit(stmt=data, number=100000) print(res)

the above outputs

0.06544675 0.035539882999999994

the use of int() and 'float()` instead of using the floor division operator and letting Python do what it does with division operator causes the code execution time to take close to 2 times longer.

I know it was done that way because of Python 2, But it needs to be updated so it can run at optimal performance in Python 3.

The above is only a single example of the kind of math found in a single function in AUI. I am sure that because of Python 2 there is more math done like this.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/wxWidgets/Phoenix/issues/1881#issuecomment-749399801, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACESNIJ37QYHUBGPAX4MGFDSWBFQFANCNFSM4VFFWFXA .

kdschlosser commented 3 years ago

Andrea.

If you have the time and the want I am willing to work on this and i would love to have the help with it. I have already sorted out the floor division problems and now I am error free in that department. I am not sure what is going on with the Fly out option. It appears to be some kind of an issue with wxTimer which would be a wxWidgets issue. I have to try and isolate the issue and remove the AGW code and see if there is a way to replicate the problem in a consistent manner. Writing the AGW AUI looks like it was one hell of an undertaking and I know it was done a long while ago. Python has changed quite a bit since then and what once the ideal way is no longer that way. wxPython has also changed and with that there is a whole other set of things to consider.

The AUI code is quite involved and for me to make changes is going to take a huge amount of effort. I guess I am hoping that you have a sharp memory. LOL.

I know that 300us is piddly in term of a performance gain. One of the things I try to pride myself on is how optimized my code is. There is a balance between resource use, speed and code readability that has to be managed just right. When I look at a programs code if there is 300us that can be gained in a single function call I then look at the number of times that function can get called over the lifetime the application is open. add them all up and put that into a pool that has other places in the code where gains can be had. I look at the application/program as a whole when optimizing. If we don't do this we end up with a program like Google Chrome and Mozilla Firefox that consume massive amounts of resources for no apparent reasons.

I just recently did an analysis of a CAN bus library. It has excessive use of exceptions requiring the user to have a lot of exception catching routines in their code. It is something on the order of 0.1us difference between exception catching and boolean evaluation. CAN can run at 1000000bit/sec, and a frame or "packet" which can be 128bits after taking 20% off for overhead if there is 100% use then there are 6250 frames a second running across the network. The time used to handle one exception catching routine ends up consuming 625us every second. and after a single hour of running 2.25 seconds has been used just to process the exception catching routine, the end result is a buffer overrun resulting in missed data. This is a real world problem with the way the library is written, most CAN interfaces only have the ability to buffer 4000 messages. that 2.25 seconds leads to 10062 missed frames every hour if the network is running at 100%.

My point being that even at 0.1us execution time difference if you step back and evaluate the application as a whole that 0.1us ends up being a bigger resource use problem then was originally thought. There is no way for the author of a library to evaluate a whole application. The author does not know how their library is being used, leaving only guessing. Actually no guessing is not the only choice, optimize the code the best way possible is the other choice. So even if there is only a 0.1us gain in execution you take it and run with it like you stole it, because in the end you have no idea how your code is being used.

kdschlosser commented 3 years ago

I have found ways of giving a speed increases to wxPython/wxWidgets. these increases allow it to keep up with CAN networks. There is some additional memory use to accomplish that kind of speed, I am not saying to update AUI in the same manner, use it for examples and ideas.

I do have a question that you may be able to answer. Is there a reason why wxPython only uses a single thread for everything that it does? I have found this single thing to be an enormous bottleneck causing lag and jitter/studder problems in the GUI. The larger issue being the event system also uses this same thread and users code gets processed in it which can cause problems outside of the control of the wxPython developers. This is something that can be managed easily enough using decorators to wrap functions, methods and classes that need to be executed in the main thread along with a way to inject the code that needs to be run into the main thread like what wx.CallAfter does.

This is one of the ways I used to speed up the GUI processing. I created a threadworker that handles processing code that can be processed outside of the main thread and handing code to the main thread to be processed that MUST be processed by it. Another thing I did was I removed all custom rendering from the event system, I used memory DC's that are only created a single time at the start of the thread and only the thread that started it uses it. I render the custom GUI element to a wxBitmap and this is where the additional memory consumption comes into play. I then use ClientDC to render the cached bitmap so the user can see it. Anything that has heavy rendering or calculations I try to make ahead of time to speed up the GUI's responsiveness.

I did see your use of list comprehension to speed an iterative function call. 👍

What do you think of using Cython to convert the Python code to C++ and compile the modules as C Extension modules (pyd)?? This would give a 50x to 200x speed boost in code execution time and is really easy to do. Making stub files so an IDE will be able to provide intellisense and type hinting is simple to accomplish as well.

lallulli commented 3 years ago

I don't know if it is related to the crash described here, but I'm experiencing a crash on AGW AUI even on sample code from official documentation.

When I drag "Pane Number One", as soon as I detach it from the left side, my application crashes on Windows, without any error messages. It works fine on Linux.

My platform is:

Operating system: Windows 10 x64 wxPython version & source: wxPython 4.1.1 (pypi wheel) Python version & source: CPython 3.9.1 32-bit (python.org)

lallulli commented 3 years ago

Update: my issue does not show up in Python 3.8.7 on Windows. Thus, it is probably related to Python 3.9. I'm opening a new issue, since it is for sure unrelated with the OP's one.

RobinD42 commented 3 years ago

Since #1896 says it does not fix the FlyOut issue I'll reopen this.

Since it is related somehow to a wx.Timer, has anybody checked if the timer is being destroyed without being stopped first?

kdschlosser commented 3 years ago

I attached the VisualStudio crash dump in my first post. This shows exactly what line of code is causing the problem.

It is happening in the event.cpp file in wx.Widgets the windows version of the file

the crash happens inside this function.

bool wxEvtHandler::SafelyProcessEvent(wxEvent& event)
{
#if wxUSE_EXCEPTIONS
    try
    {
#endif
        return ProcessEvent(event);
#if wxUSE_EXCEPTIONS
    }
    catch ( ... )
    {
        WXConsumeException();

        return false;
    }
#endif // wxUSE_EXCEPTIONS
}

specifically this line

return ProcessEvent(event);

ProcessEvent is a Windows API function. If memory serves it is an access violation error that is taking place. So maybe the "event" object that is being passed to ProcessEvent was created in a different thread? or for some reason is not the correct data type?

This problem does happen running Python 3.8 for me. I have not checked 3.7. IDK if there have been any changes made to wxTimer recently that would cause this kind of an issue.

kdschlosser commented 3 years ago

@RobinD42

I will check the code in agw aui and make sure that the timer isn't getting closed at the wrong time. If it happens that it is getting closed then perhaps some kind of an exception can be raised that wouldn't crash the application should be added.

RobinD42 commented 3 years ago

ProcessEvent is a Windows API function

No, it's actually a method of wxEvtHandler. They would have used ::ProcessEvent to call the Windows API.

The typical problem for crashes related to timers is that a timer event from the system arrives after the widget the timer is associated with has been destroyed. In other words, the C++ this pointer in the code snippet you give above is now garbage and when this->ProcessEvent(event); is called, it crashes. wxWidgets doesn't track that association, and probably can't with the current architecture, so it is up to the programmer to ensure that timers are stopped and can't generate more events.

If you're not able to find the right place to stop the timers then one workaround that would probably work would be to not make that association. In other words, do not pass the owner arg to the timer's __init__ or call SetOwner. For handling the timer event you would not be able to use:

    self.Bind(wx.EVT_TIMER, ...)

where self is what you would have used as the owner before. Instead you can make a new class derived from wx.Timer and override the Notify method.

class MyTimer(wx.Timer):
    def Notify(self):
        do_something()

Since wx.Timer is a wx.EvtHandler then you can call its Bind to bind a handler directly to the timer instead of to some widget. In this case and the Notify case above you'll need to take care that whatever widgets you access from the timer handler still exists, something like if myframe and not myframe.IsBeingDeleted(): ...

kdschlosser commented 3 years ago

@RobinD42

This is not something I would normally do.


class Frame(wx.Frame):

    def __init__(self):
        wx.Frame.__init__(self, None, -1)
        button = wx.Button(self, wx.ID_OK)
        self.Bind(wx.EVT_BUTTON, self.on_ok)

    def on_ok(self, evt):
        pass

this is what I normally do.


class Frame(wx.Frame):

    def __init__(self):
        wx.Frame.__init__(self, None, -1)
        button = wx.Button(self, wx.ID_OK)
        button.Bind(wx.EVT_BUTTON, self.on_ok)

    def on_ok(self, evt):
        pass

Is this the kind of thing you are talking about?