Fix #514

[tokejepsen]

What's changed? Store QApplication at module level and always exec.

Spawned from https://github.com/getavalon/core/pull/520

This has been tested standalone and in Maya/Nuke.

[BigRoy]

In essence this would still be wrong - there's no need to store the QApplication. Wasn't that what we got from the discussion and looking into the issue?

The QApplication.instance() method will always return the singleton instance, however in some cases just exec_() had to trigger again which you're now doing.

So wouldn't you only need to move the call to app.exec_()?

[tokejepsen]

Wasn't that what we got from the discussion and looking into the issue?

What I'm trying to fix here is the segfault that happens when using the the tools standalone, which I would imagine is the same issue as described in #514 I was not able to hold onto the QApplication on a tool level successfully, so this is the solution.

[BigRoy]

What I'm trying to fix here is the segfault that happens when using the the tools standalone, which I would imagine is the same issue as described in #514 I was not able to hold onto the QApplication on a tool level successfully, so this is the solution.

What's the easiest way for me to reproduce this segfault on my end? Can I reproduce this in standalone Python?

Any python Qt library should always cling on to the actual QApplication instance - it should somehow keep that reference unless maybe you'd reload the Qt libraries. :) I still feel, as I described there, that the actual segfault happens due to accessing the closed Qt widget with module.window for a Window that is actually at that point in time on a quited/closed QApplication. Meaning the problem is not on the QApplication but on the Window widget. Sorry for being so pushy on this. I'd just like to figure out why exactly this is happening.

[BigRoy]

@tokejepsen does this behave the same?

Run just in a Python command line interpreter.

# assumes a running QApplication instance
from PyQt5 import QtWidgets, QtCore

app = QtWidgets.QApplication([])

window = QtWidgets.QDialog()
window.show()

app.exec_()

# this will crash, similar to what we're doing with `module.window`
window.show()

# note that this line below isn't even reached...
# because it crashes on the window.show()
# in this example both `app` and `window` variable persist
# but the crash is due to window having been destroyed...
app.exec_()

1. Window works fine.
2. Close the window.
3. This closes down the QApplication (but instance persists - also because it's a variable)
4. Show the closed window... that is a destroyed/invalid QObject now due to QApplication shutting down.

This seems like the problem you're running into instead.

Maybe we should add on destroyed callback that removes itself from module.window and setting it back to None to ever avoid accessing the dangling pointer to the destroyed window? That would be the only safe way to remove the dangling reference, according to this: https://stackoverflow.com/a/32218542/1838864

[tokejepsen]

does this behave the same?

Tried with "your" code below and got the window to how twice but the python process is stuck.

import sys
import contextlib
# assumes a running QApplication instance
from avalon.vendor.Qt import QtWidgets

@contextlib.contextmanager
def application():
    app = QtWidgets.QApplication.instance()

    if not app:
        print("Starting new QApplication..")
        app = QtWidgets.QApplication(sys.argv)
        yield app
        app.exec_()
    else:
        print("Using existing QApplication..")
        yield app

with application():
    app = QtWidgets.QApplication([])

    window = QtWidgets.QDialog()
    window.show()

    app.exec_()

    window.show()

    # note that this line below isn't even reached...
    # because it crashes on the window.show()
    # in this example both `app` and `window` variable persist
    # but the crash is due to window having been destroyed...
    app.exec_()

This is not what happens when I see the segfault. The python process crashes and exits.

What's the easiest way for me to reproduce this segfault on my end? Can I reproduce this in standalone Python?

I honestly dont know how and this is clearly going way over my head and comprehension. All I can say it that it makes the Photoshop integration which I'm working on, not error out, without affecting Nuke/Maya.

Maybe the title is wrong and I shouldn't assume it'll fix #514.

[BigRoy]

Funny that you say it now crashes on you too. I'm trying it again now... and it works fine. Ha, what is going on? It's as if the Window doesn't get destroyed/garbage collected in time one some occassions and it remains valid then?

Anyway... can we add a comment above the line where store the QApplication into self.app as to why we're doing it, and potentially mentioning #514 so it's clear as to why it's there. If it helps you to continue we can always investigate later as to what exactly is going on.

@jasperges could you see whether this fixes your issues too? Specifically #514 ?

[tokejepsen]

I have noticed just now that the Loader tool handles an existing module window badly; https://github.com/getavalon/core/blob/master/avalon/tools/loader/app.py#L389-L408

This causes the Loader to become unresponsive on second showing.

[jasperges]

@jasperges could you see whether this fixes your issues too? Specifically #514 ?

@BigRoy I guess so, I will have to check with @kguyaux and his TVPaint integration.

[tokejepsen]

Have put the execution of existing QApplications behind the environment variable AVALON_QTAPPLICATION_EXEC. How do we feel about this PR? This is still needed for #524

[BigRoy]

I think the change made in this commit with the environment variable actually also would be intended for the first start of the QApplication. If I'm not mistaken, then for Blender and Cinema4D you should never call .exec_() as it holds the thread and will freeze the host application. Right @jasperges ? It's super weird.

Previously I worked around it by creating an Instance before calling any of the Avalon tools as that would mean it would always skip trying to call exec_(). Maybe it makes sense to place the environment variable for first creation of QApplication too so the "hack" isn't needed?

---

Other than that, I can live with this PR. It's one of those oddities I don't really understand but it seems to fix things for your use case so I guess it's a required change.

[tokejepsen]

Previously I worked around it by creating an Instance before calling any of the Avalon tools as that would mean it would always skip trying to call exec_(). Maybe it makes sense to place the environment variable for first creation of QApplication too so the "hack" isn't needed?

Hmm, I think I should maybe follow your example and just enforce the QApplication for the Photoshop integration only.

This PR does not have enough grounding, and it affects too much code to just add. Closing this and fixing in #524