Opened 4 years ago

Closed 4 years ago

#15545 closed task (fixed)

Document TOR_PT_EXIT_ON_STDIN_CLOSE in the pt-spec.

Reported by: yawning Owned by: yawning
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Circumvention/Pluggable transport Version:
Severity: Normal Keywords: pt-spec, pt-wants, 028-triaged, pre028-patch
Cc: dcf, asn, yawning Actual Points:
Parent ID: #16754 Points: small
Reviewer: Sponsor:

Description

This is the ticket for the documentation side of the TOR_PT_EXIT_ON_STDIN_CLOSE and associated behavior that was implemented as part of #15435.

pt-spec.txt needs to document that if the env var is set to 1, then PTs should assume that tor will keep stdin open, and to treat stdin being closed as the same as a SIGTERM (graceful shutdown immediately).

Child Tickets

Change History (12)

comment:1 Changed 4 years ago by yawning

Status: newneeds_revision

Interesting, the current revision of pt-spec.txt doesn't mention the SIGINT behavior at all. I guess this is ok, since it's not actually implemented in the tor side code, and I don't really see a good case for keeping it.

Anyway, this documents the PT termination behavior, including the shutdown on close stuff:
https://github.com/Yawning/torspec/compare/bug15545

My plans going forward to bring some sanity to the PT shutdown process, now that TOR_PT_EXIT_ON_STDIN_CLOSE exists is to change the tor side PT teardown code to:

  1. Close stdin (and on U*IX, send a SIGTERM, PT behavior here is equivalent).
  2. Wait for a grace period (~1 sec?)
  3. If the child still is not dead, send a SIGKILL/TerminateProcess().

I think that behavior reflects what's in the spec, is far more portable/sane than what currently exists, and will fix #9330 since PTs that have their own children will have ample time to clean up.

comment:2 Changed 4 years ago by yawning

Status: needs_revisionneeds_review

Repushed to use dcf's wording from #9732 (which my commit also fixes).

comment:3 Changed 4 years ago by yawning

#9741 should also probably go away as a wontfix if we're giving up on the SIGINT thing.

comment:4 in reply to:  1 Changed 4 years ago by dcf

Replying to yawning:

Anyway, this documents the PT termination behavior, including the shutdown on close stuff:
https://github.com/Yawning/torspec/compare/bug15545

There are some typo fixes that should be a separate commit.

+ 2.3. Extended ORPort
- versions of this configuration protocol Tor supports. Clients
+ versions of this configuration protocol Tor supports. Proxies
- too while performing the managed proxy protocol:
+ to while performing the managed proxy protocol:

A little typo "enviornment" (three times).

Instead of saying "by closing their listener ports, closing all existing connections, running other cleanup as required, and terminating" twice in two separate paragraphs, it's easier to understand if you first say what should happen on SIGTERM, and then say that if TOR_PT_EXIT_ON_STDIN_CLOSE=1, that stdin EOF has the same meaning as SIGTERM.

comment:5 Changed 4 years ago by nickm

Status: needs_reviewneeds_revision

comment:6 Changed 4 years ago by yawning

Parent ID: #16754

comment:7 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

comment:8 Changed 4 years ago by nickm

Keywords: 028-triaged added

comment:9 Changed 4 years ago by nickm

Cc: yawning added
Points: small

comment:10 Changed 4 years ago by nickm

Keywords: pre028-patch added

comment:11 Changed 4 years ago by nickm

Severity: Normal

yawning: does your pt rewrite make this ticket obsolete?

comment:12 Changed 4 years ago by yawning

Resolution: fixed
Status: needs_revisionclosed

Indeed, the rewritten pt-spec includes documenting this behavior.

Note: See TracTickets for help on using tickets.