Opened 5 years ago

Closed 4 years ago

#3948 defect closed fixed (fixed)

wxdemo.py does not exit

Reported by: ciphergoth Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: itamar, khorn, jesstess, thijs, riteshn@… Branch: branches/wxreactor-stop-fix-3948
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

On Ubuntu "Jaunty Jackalope" i386, using the system packages for wxPython 2.8.9.1 and Twisted 8.2.0, I run

python /usr/share/doc/twisted-doc/examples/wxdemo.py

The demo window appears, but clicking on "File > Exit" or the window manager close button has no effect. Expected behaviour: the process should terminate.

I'm having trouble getting my own wxreactor-based application to exit properly, so a working demo that shows the proper way to do it would be much appreciated - thanks!

Attachments (1)

issue_3948.patch (1.2 KB) - added by psykidellic 4 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 5 years ago by ciphergoth

  • Owner glyph deleted

comment:2 Changed 5 years ago by jesstess

I had to add self.Destroy() before reactor.stop() in DoExit() to make the window close on clicking "File > Exit", fwiw, but the window manager close button still doesn't work and the process still doesn't terminate either way.

comment:3 Changed 5 years ago by michael-fig

I've confirmed this under hardy heron (Ubuntu 8.04) with Twisted-9.0.0 compiled from sources.

Even with a Destroy() added before the reactor.stop(), the process hangs (one thread is still active) and cannot be killed except with kill -9.

I am happy to help debug this, as it is a showstopper that will prevent me from using Twisted on my next project.

comment:4 follow-up: Changed 5 years ago by michael-fig

I think I found it...

The wxreactor was just calling _threadselect.stop directly, when it needed to post an event to its queue and wake it instead. This seems to solve everything (I will test more thoroughly later, but for now it gets the wxdemo working without any trouble).

Here's the replacement wxreactor.py WxReactor.stop():

    def stop(self):
        """
        Stop the reactor.
        """
        if self._stopping:
            return
        self._stopping = True
        self.callFromThread(_threadedselect.ThreadedSelectReactor.stop, self)

comment:5 Changed 5 years ago by khorn

  • Cc khorn added

comment:6 in reply to: ↑ 4 ; follow-up: Changed 5 years ago by michael-fig

Replying to michael-fig:

I think I found it...

I would also suggest adding a note to the Twisted GUI documents: if you want to show a wx dialog from within a reactor callback, you should use wx.CallLater(myDialog.ShowModal), otherwise you'll hang the reactor until the reactor callback returns.

After discovering this tip, I'm pleased to say that the wxreactor works as expected and I'll be using it after all!

comment:7 in reply to: ↑ 6 Changed 5 years ago by michael-fig

Here is code to monkey-patch the wxreactor if you don't have the ability to change the sources. It uses a more correct technique, in which the stop method itself is called from the main thread:

from twisted.internet import wxreactor
wxreactor.install()
from twisted.internet import reactor

# Monkey-patch the wxreactor to make it exit correctly.
if wxreactor.WxReactor.callFromThread is not None:
    oldStop = wxreactor.WxReactor.stop
    def stopFromThread(self):
        self.callFromThread(oldStop, self)
    wxreactor.WxReactor.stop = stopFromThread

comment:8 Changed 5 years ago by jesstess

  • Cc jesstess added

comment:9 Changed 5 years ago by thijs

  • Cc thijs added

comment:10 Changed 4 years ago by itamar

  • Keywords review added

We had another person with same problem, and the patch fixed things for them. Moving this to review, since there's a patch and it seems to work.

comment:11 Changed 4 years ago by Screwtape

I'm confused. There's no patch attached to this ticket, and no branch. Exactly what needs to be reviewed?

comment:12 Changed 4 years ago by itamar

Comment #4 is almost a patch.

comment:13 Changed 4 years ago by exarkun

  • Keywords review removed
  • Owner set to michael-fig

I can't reproduce the working behavior reported by making the change in comment 4. I also thought about the change for a while before testing it, and I could not think of a reason it would make a difference.

Changed 4 years ago by psykidellic

comment:14 Changed 4 years ago by psykidellic

  • Cc riteshn@… added
  • Keywords review added
  • Owner michael-fig deleted

Find attached a complete patch - fix from comment 4 and top files - that fixes this issue. Using wx 2.8.10 on Ubuntu.

Basically, we need to stop the threaded wxreactor.stop from the main reactor thread rather than itself.

I really dont know how to write a test case for it but our internal GUI app (which is considerably complex as it uses bunch of custom controls) seems to work.

So if you have any suggestion on how to create one, I can do that - otherwise I really dont know how to write a testcase for this.

Also, lot of methods didnt have the 2 line breaks as mentioned in the styling guide, so I updated the same with this.

This patch applies to trunk at 30649.

comment:15 Changed 4 years ago by psykidellic

FWIW, the problem is there on both Linux and Mac OS X using wx 2.8.10. And the changes fixes both the environment.

comment:16 Changed 4 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/wxreactor-stop-fix-3948

(In [30728]) Branching to 'wxreactor-stop-fix-3948'

comment:17 Changed 4 years ago by exarkun

(In [30729]) Make sure threadedselectreactor.stop() is noticed by waking up all the reactor threads when that method is called.

refs #3948

comment:18 Changed 4 years ago by exarkun

(In [30731]) news fragment

refs #3948

comment:19 Changed 4 years ago by itamar

  • Keywords review removed
  • Owner set to exarkun

wxdemo.py now exits correctly - merge.

comment:20 Changed 4 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(In [30740]) Merge wxreactor-stop-fix-3948

Author: exarkun
Reviewer: itamar
Fixes: #3948

Wake up the necessary threads in wxreactor when the reactor is stopped to ensure
the reactor stops in a timely manner.

comment:21 Changed 4 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.