Opened 8 years ago

Closed 23 months ago

#3701 closed defect (wontfix)

PyTorCtl shouldn't os.kill() itself

Reported by: mikeperry Owned by:
Priority: High Milestone:
Component: Core Tor/Torflow Version:
Severity: Blocker Keywords:
Cc: aagbsn@… Actual Points:
Parent ID: #13630 Points:
Reviewer: Sponsor:

Description

In order to ensure we died properly from uncaught exceptions in user-written event threads, I hacked in an os.kill() into the event loop that catches these exceptions, because sys.exit() only exits the thread, and os._exit() doesn't call atexit handlers. os.kill() does call the atexit handlers, but it causes the return value of the interpreter to be the signal used. This exit value breaks out bw authority parent/child model, which relies on exit codes to properly detect child death and respawn.

Child Tickets

Change History (6)

comment:1 Changed 8 years ago by aagbsn

Here are some options I've thought of:

  1. Make main thread aware of the child thread state (via signals) and exit when appropriate.
    • pro: Could be implemented with a custom signal/handler and os.kill() fairly trivially.
    • con: not generic; may require programs that use TorCtl to implement signal handling.
  1. Allow BwAuthority to die when TorCtl fails. This has been the default behavior.
    • pro: current behavior, requires no change
    • con: BwAuthority dies.
  1. Do not exit BwAuthority when bwauthority_child is killed with SIGTERM. (Restart child)
    • pro: trivial to implement
    • con: makes bwauthority a pain to kill, though could modify run_scan.sh to use kill -9

comment:2 Changed 8 years ago by aagbsn

Cc: aagbsn@… added

comment:3 Changed 8 years ago by mikeperry

My recommendation is approach #3, but to also change the signal sent from pytorctl from SIGTERM to something else that will still cause the interpreter to kill itself. SIGPIPE, or SIGABRT perhaps? Or maybe just an assert?

We can think about fixing this the right way later.. The important thing now is to make sure the bw auth parent doesn't exit from it.

comment:4 Changed 8 years ago by mikeperry

In fact, perhaps we should just make a separate ticket for hacking the bw auth parent to deal with this signal better. Unless an assert works on the TorCtl side (instead of os.kill()) to get us what we want (death from the child, and the parent just restarting a new one).

comment:5 Changed 7 years ago by atagar

Component: TorctlTorflow

Discussion is related to TorFlow. Reassigning to that component.

comment:6 Changed 23 months ago by teor

Parent ID: #13630
Resolution: wontfix
Severity: Blocker
Status: newclosed

This is a bugfix that will be obsoleted by the new bwauth replacement project, see #13630.

Note: See TracTickets for help on using tickets.