Make signal handlers optional, for starting Tor in-process

When Tor starts, it installs handlers for a bunch of signals. But if you're running Tor in its own thread, there's a good chance you don't actually want that behavior.

changed milestone to %Tor: 0.3.2.x-final

added component::core tor/tor milestone::Tor: 0.3.2.x-final owner::nickm parent::23684 priority::medium resolution::implemented review-group-30 s8-api severity::normal sponsor::8 status::closed tor-mobile type::defect labels

I've documented this feature in a torspec branch called disable_signal_handlers.

The implementation is in a tor branch called disable_signal_handlers.

Trac:
Status: new to accepted
Owner: N/A to nickm

Trac:
Status: accepted to needs_review

Trac:
Keywords: N/A deleted, review-group-30 added
Milestone: Tor: 0.3.4.x-final to Tor: 0.3.2.x-final

Why is this listed for controllers in control-spec.txt if you can't SETCONF/change the value while Tor is running (as per the check added to options_transition_allowed)?

If there is another way for controllers to use this other than SETCONF, probably the control-spec.txt should say to do that (and not SETCONF). If not, then this should go into the manpage (and probably should go into the manpae anyway).

I was also briefly confused what the event handler was doing in the else where signal handlers are set to disabled. Maybe document that it is for the SIGNAL control port command/activate_signals()?

I think the rest of this looks OK though. Tests pass w/ hardening, spaces/changes OK, etc.

Trac:
Status: needs_review to needs_revision

Why is this listed for controllers in control-spec.txt if you can't SETCONF/change the value while Tor is running (as per the check added to options_transition_allowed)?

That's because it's meant for use by controllers only, not in a regular torrc. If you're not embedding Tor, this option isn't useful. We already have similar controller-only unchangeable options in control-spec.txt, like OwningControllerFD.

I was also briefly confused what the event handler was doing in the else where signal handlers are set to disabled. Maybe document that it is for the SIGNAL control port command/activate_signals()?

Okay! I've added a commit to improve the documentation in general.

Trac:
Status: needs_revision to needs_review

Ok, this documentation for the code looks great. I still think it is surprising to list things in controller-spec.txt that can't be set via SETCONF. The first paragraph of that section says "These options can be set and examined by the SETCONF and GETCONF commands.."

How about this fixup so that it is not as much of a surprise: https://gitweb.torproject.org/user/mikeperry/torspec.git/commit/?h=disable_signal_handlers&id=0d95894f3ae46009d0e3f5dddcbe3f221d8a6f1e

That branch also has a bonus commit for __OwningControllerFD: https://gitweb.torproject.org/user/mikeperry/torspec.git/commit/?h=disable_signal_handlers&id=cde44d7276bef1ec4c538b5f585317c4a1ddeca5

Great! I squashed those in, and added d6ff5e71e305f5baef1df6676e0690be6ba705c9 to fix the first paragraph. I've merged that to torspec master, and merged the code branch to Tor master. Thanks for the review!

Trac:
Resolution: N/A to implemented
Status: needs_review to closed

closed

moved to tpo/core/tor#24588 (closed)