[tor-bugs] #3473 [Pluggable transport]: Better signal handling for obfsproxy

[Tor Bug Tracker & Wiki]

#3473: Better signal handling for obfsproxy
---------------------------------+------------------------------------------
 Reporter:  asn                  |          Owner:  asn         
     Type:  defect               |         Status:  needs_review
 Priority:  normal               |      Milestone:              
Component:  Pluggable transport  |        Version:              
 Keywords:                       |         Parent:              
   Points:                       |   Actualpoints:              
---------------------------------+------------------------------------------

Old description:

> We currently treat SIGINT and SIGTERM equally, by terminating obfsproxy
> asap.
>
> 180-pluggable-transport.txt dictates that:
> '''
>   Proxies should respond to a single INT signal by closing their
>   listener ports and not accepting any new connections, but keeping
>   all connections open, then terminating when connections are all
>   closed.  Proxies should respond to a second INT signal by shutting
>   down cleanly.
> '''
>
> I think we should conform SIGINT with the 180 spec, and leave SIGTERM as
> is.

New description:

 We currently treat SIGINT and SIGTERM equally, by terminating obfsproxy
 asap.

 180-pluggable-transport.txt dictates that:

   Proxies should respond to a single INT signal by closing their
   listener ports and not accepting any new connections, but keeping
   all connections open, then terminating when connections are all
   closed.  Proxies should respond to a second INT signal by shutting
   down cleanly.

 I think we should conform SIGINT with the 180 spec, and leave SIGTERM as
 is.

--

Comment(by nickm):

 Instead of (or in addition to) the shutting_down flag, I think it's better
 to close the listeners when we're shutting down, rather than just make
 them close their sockets immediately.

 Did you test this?  The cl_remove in conn_free() comes right after the
 memset that wrecks the contents of conn.

 Why have a separate conn_list_t type; why not just add the 'next' pointer
 to conn_t?

 For efficient removal, the list should probably be doubly-linked.

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