Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#5095 closed defect (fixed)

Setting __ReloadTorrcOnSIGHUP=0 breaks logrotate

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Vidalia copied a feature from TorK in its 0.2.16 release:

  o Sets __ReloadTorrcOnSIGHUP to 0 if SAVECONF failed, which means
    the user can't write the torrc file. Fixes bug 4833.

The bug that it fixed was that if Vidalia does a saveconf that fails (e.g. because Tor can't write to /etc/tor/torrc), and Vidalia has setconf something different from the torrc, then Vidalia will setconf ReloadTorrcOnSIGHUP to 0 to prevent a silent and confusing clobbering of Tor's config when the daily hup happens.

The bug that it introduces, though, is that this daily hup typically involves a logrotate, and now hup is mysteriously not going to open any new logs.

(I'm not sure which milestone to set here -- it's a bug that will affect Tor 0.2.2 also, but I'm wary of invasive fixes on 0.2.2 at this point.)

Child Tickets

Change History (7)

comment:1 Changed 8 years ago by arma

One option would be for do_hup to run the same stuff options_act_reversible() does to logs, in the ReloadTorrcOnSIGHUP is 0 case. After all, the config option tells us not to reload the torrc, not to avoid doing other smart things on hup.

That change might be non-invasive enough to make it into 0.2.2 -- especially since the behavior would only change for users who set ReloadTorrcOnSIGHUP to 0.

comment:2 Changed 8 years ago by nickm

Status: newneeds_review

Please review and test branch "bug5095" in my public repository. I tried it a little as a client, but for all I know it makes servers explode.

comment:3 Changed 8 years ago by arma

1) char *msg; wants to be char *msg = NULL; yes?

2)

-  if (old_options) {
+  if (old_options && old_options != global_options) {

is just an optimization, since option_is_same() will be true at each step, yes? (It's fine as an optimization; I just want to make sure I'm not missing something deeper.)

Looks fine other than that (I think -- it definitely needs some testing too).

comment:4 in reply to:  3 Changed 8 years ago by nickm

Replying to arma:

1) char *msg; wants to be char *msg = NULL; yes?

Yes.

2)

-  if (old_options) {
+  if (old_options && old_options != global_options) {

is just an optimization, since option_is_same() will be true at each step, yes? (It's fine as an optimization; I just want to make sure I'm not missing something deeper.)

It's an optimization.

Looks fine other than that (I think -- it definitely needs some testing too).

Added a fixup commit for issue 1; I hope folks will test. :)

comment:5 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Seems to work for me; I'll merge it.

comment:6 Changed 7 years ago by nickm

Keywords: tor-client added

comment:7 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.