pyapp-kit / superqt

Missing widgets and components for Qt-python
https://pyapp-kit.github.io/superqt/
BSD 3-Clause "New" or "Revised" License
210 stars 38 forks source link

Nested Workers are Failing Silently #64

Closed michael-sutherland closed 2 years ago

michael-sutherland commented 2 years ago

🐛 Bug

If you create a worker and then that worker creates another worker, the function isn't called and it fails silently. I'm not sure if nested workers is supposed to be supported or not, but it shouldn't be failing silently either way.

To Reproduce

Steps to reproduce the behavior:

  1. Create a worker
  2. Create another worker from that worker
  3. Nested worker never gets called and there is no exception raised
import napari
from napari.qt.threading import create_worker
from qtpy.QtWidgets import QWidget, QVBoxLayout, QPushButton

class TestWidget(QWidget):
    def __init__(self):
        super().__init__()
        # setup simple GUI elements
        layout = QVBoxLayout()
        self.test1_btn = QPushButton("No Workers", self)
        self.test1_btn.clicked.connect(self.test1)
        layout.addWidget(self.test1_btn)
        self.test2_btn = QPushButton("1 Worker", self)
        self.test2_btn.clicked.connect(self.test2)
        layout.addWidget(self.test2_btn)
        self.test3_btn = QPushButton("2 Nested Workers", self)
        self.test3_btn.clicked.connect(self.test3)
        layout.addWidget(self.test3_btn)
        # activate layout
        self.setLayout(layout)

    def test1(self):
        print("hello from test1")

    def test2(self):
        worker = create_worker(self._test2)
        worker.start()

    def _test2(self):
        print("hello from test2")

    def test3(self):
        worker = create_worker(self._test3a)
        worker.start()

    def _test3a(self):
        worker = create_worker(self._test3b)
        worker.start()

    def _test3b(self):
        print("hello from test3")

viewer = napari.Viewer(title="test threading")
test_widget = TestWidget()
viewer.window.add_dock_widget(test_widget, name="test widget", area="right")

napari.run()

Expected behavior

I expected to to be able to create a worker from within another worker. Ideally, any functions that connect on callback would be executed in the GUI thread, since the original worker may be finished with its task and needs to be made available in the worker pool. Also, it would be nice to have an "easy" path back to the GUI thread in this case. If nested workers isn't supported, then I expected calling start on the nested worker would throw an exception.

Environment

OpenGL:

Screens:

Plugins:

Additional context

tlambert03 commented 2 years ago

Hi @michael-sutherland, thanks for the very useful example. This can be fixed directly calling start_() in the worker start method here, rather than adding it to the Queue with QTimer.singleshot() (since you need an event loop to run a QTimer and there isn't one in a thread)

        start_ = partial(QThreadPool.globalInstance().start, self)
        QTimer.singleShot(10, start_)

however, I need to dig a little to remember why that queuing was important in the first place (should have commented it at the time! :joy:). thanks

tlambert03 commented 2 years ago

fixed in https://github.com/napari/superqt/pull/63

michael-sutherland commented 2 years ago

Thanks! Quick question: If you connect to an event in the nested thread, which thread does the callback get called in? Gui Thread --> Worker thread --> Nested worker thread with returned.connect set to "callback" Which thread executes "callback"?

tlambert03 commented 2 years ago

good question! I could wager a guess, but we should just directly test it

michael-sutherland commented 2 years ago

Callback is in the GUI thread. Very nice! Thanks for fixing this so fast!

tlambert03 commented 2 years ago

awesome! thanks for checking!

michael-sutherland commented 2 years ago

I think there is still an issue with multiple threads, although I'm not sure how to reproduce it in a small example. I'm setting a callback (returned.connect) in a nested thread in my application and the callback isn't being called. If I remove the first level worker and just call the function directly, so that there is only a single worker (formerly the nested worker), the callback from returned.connect is called successfully. However, my toy example from above with callbacks added doesn't have the same issue, the nested workers call the returned.connect callbacks just fine! If I figure out how to isolate the issue, I'll post the smaller sample. Currently, my full code is rather cumbersome and requires a connection to a remote server to run.