xpra icon
Bug tracker and wiki

Opened 4 years ago

Closed 4 years ago

#896 closed enhancement (fixed)

get rid of daemon threads

Reported by: Antoine Martin Owned by: alas
Priority: major Milestone: 0.16
Component: core Version: 0.15.x
Keywords: Cc:

Description

As per this answer to Python Exception in thread Thread-1 (most likely raised during interpreter shutdown)?, we should not be using daemon threads and should just ask them to exit cleanly during the cleanup phase.

Change History (5)

comment:1 Changed 4 years ago by Antoine Martin

Milestone: 0.170.16
Status: newassigned
  • r9698 prepares for the change
  • r9700 changes all but the network IO threads to non-daemon mode
  • r9701 debugging tweaks and attempts to set socket to non-blocking (unfortunately, this does not affect the in progress recv call..)

Mostly there - only the two network threads can end up not exiting due to blocking IO calls.

(somewhat similar to #873)

Last edited 4 years ago by Antoine Martin (previous) (diff)

comment:2 Changed 4 years ago by Antoine Martin

Owner: changed from Antoine Martin to alas
Status: assignednew

Server fixes and improvements:

  • The changes above could cause the server to get stuck on control-C, because the main loop is no longer running and we relied on it to cause the encode threads to exit. This is fixed in r9711 - (may get backported).
  • r9710 also dumps all the threads still running during the server cleanup (with -d server) - there should only be network IO threads left (and the main thread)
  • r9714 should ensure we always eventually force_disconnect the connections if they haven't cleaned themselves up following the disconnect_protocol

@afarr: this is just a FYI - feel free to close as an ACK. If I've done this wrong, it may be possible for the server to get stuck and not exit when we tell it to:

  • via control-C
  • via kill -SIGINT $THESERVERPID
  • via xpra exit or xpra shutdown
  • when the server exits because of settings like exit-with-client or exit-with-children

It just makes the code more correct and robust, and may avoid some of the exceptions that can appear during shutdown (like #873).

Last edited 4 years ago by Antoine Martin (previous) (diff)

comment:3 Changed 4 years ago by Antoine Martin

Another ticket that keeps on giving... more fixes in r9851 + r9852.

(and also some more cosmetic improvements in r9849)

comment:4 Changed 4 years ago by alas

Tried with fedora 20 & 21 servers, 0.15.4 r9951, r10082, r10133 ... control-c, kill -15 PID, and a simple xpra stop :DISPLAY. Everything closed as expected, servers re-started smoothly.

Trying xpra exit, however, the server indicated it had shutdown, but when trying to start again it failed due to an unknown session at :DISPLAY... and I had to kill the Xorg PID to clean the display up.

As for xpra shutdown, I couldn't figure out the syntax for it. The wiki seems to indicate it's a client sent message, but I couldn't figure out how to send from the client, and it wasn't a recognized control channel message.

comment:5 Changed 4 years ago by Antoine Martin

Resolution: fixed
Status: newclosed

Trying xpra exit, however, the server indicated it had shutdown, but when trying to start again it failed due to an unknown session at :DISPLAY... and I had to kill the Xorg PID to clean the display up.


That's because it is exactly what it does. See the manual: This command attaches to a running xpra server, and requests that it terminates immediately. Unlike xpra stop, the Xvfb process and its X11 clients (if any) will be left running.

After using this command, you can start a new xpra server against the display it left behind using the --use-display command line argument.


As for xpra shutdown, I couldn't figure out the syntax for it.


There is no such command, I meant "stop".
(there is an internal command called 'shutdown' which is the one used by "stop")

Closing.

Note: See TracTickets for help on using tickets.