[tor-bugs] #15125 [meek]: meek-client-wrapper does not use signals well

[Tor Bug Tracker & Wiki]

#15125: meek-client-wrapper does not use signals well
---------------------------+-----------------
     Reporter:  infinity0  |      Owner:  dcf
         Type:  defect     |     Status:  new
     Priority:  normal     |  Milestone:
    Component:  meek       |    Version:
   Resolution:             |   Keywords:
Actual Points:             |  Parent ID:
       Points:             |
---------------------------+-----------------
Changes (by dcf):

 * owner:  asn => dcf
 * component:  Pluggable transport => meek


Comment:

 Replying to [ticket:15125 infinity0]:
 > - it does not respond to SIGINT or SIGKILL.

 ? It does SIGINT and SIGTERM [https://gitweb.torproject.org/pluggable-
 transports/meek.git/tree/meek-client-torbrowser/meek-client-
 torbrowser.go?id=0.15#n160 right here]:
 {{{
         signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
 }}}
 And I thought SIGKILL "cannot be caught, blocked, or ignored"?

 > also, the signal handling code is different from meek-client. perhaps we
 should move it to goptlib?

 That actually is the reason it's not in goptlib—I have had to do the
 signal handling code in different ways in different programs. The program
 might want to close sockets, or log something, or whatever. I didn't find
 a good way to abstract it (that is significantly more expressive than just
 handling the signals explicitly).

 > - it uses sigkill to kill its children, not giving them a chance to
 clean up. Yes, this is awkward on windows but we can at least do something
 nicer on posix systems.

 That's a fair point. However keep in mind that the code already passes on
 any signal it receives; so if we got a SIGTERM, so will the child
 processes. I guess I might prefer to keep the destructive kill, but only
 do it if we haven't sent some other signal to a child process (rather than
 unconditionally as a defer as we do now).

 I saw your
 [https://github.com/infinity0/meek/commit/ae0dc0ff30b332955cc46e228ca3558f24602941
 ae0dc0ff] ("give child processes a chance to clean up"). I am strongly
 disinclined to do anything involving a timer. Do signals really work like
 that? The parent process needs to still be running in order for the signal
 to be delivered?

 I'm skeptical that closing stdin is sufficient to stop Firefox. Does that
 work on Windows?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/15125#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online