Opened 19 months ago

Closed 6 months ago

#25613 closed enhancement (fixed)

Close child's stdin to signal exit in meek-client-torbrowser

Reported by: dcf Owned by: dcf
Priority: Low Milestone:
Component: Circumvention/meek Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: cohosh Sponsor:

Description

Moved from #24642.

Another (possibly better) option is to call cmd.StdinPipe() and just never close the pipe (that way the child process's stdin is separate from the parent's, so you don't have a race between them trying to terminate when the stdin is closed).

The bug24642 branch uses the StdinPipe idea.

I'm not totally happy with it, because ideally according to pt-spec, we should keep track of the stdin handle, and close it before sending anything like SIGTERM to the subprocess. But that would require more rewriting and is more than you need right now.

Child Tickets

Change History (5)

comment:1 Changed 6 months ago by dcf

Status: newneeds_review

Branch bug25613 refactors the way that meek-client-torbrowser handles its client processes.

https://gitweb.torproject.org/pluggable-transports/meek.git/log/?h=bug25613&id=36d06b5f9a0d0c4a7aa251f33460886f68a2ab22
https://gitweb.torproject.org/pluggable-transports/meek.git/diff/?h=bug25613&id=36d06b5f9a0d0c4a7aa251f33460886f68a2ab22&id2=29797a33ecee1c6fc4882fc2b6beabcd64f554ce

This is the commit that touches meek-client. We get a handle to the child's stdin:

// ptCmd is a *exec.Cmd augmented with an io.WriteCloser for its stdin, which we
// can close to instruct the PT subprocess to terminate.
type ptCmd struct {
	*exec.Cmd
	StdinCloser io.WriteCloser
}

then close it as the first step of terminating the process.

	err := cmd.StdinCloser.Close()

comment:2 Changed 6 months ago by cohosh

Reviewer: cohosh

comment:3 Changed 6 months ago by cohosh

Status: needs_reviewmerge_ready

Looks good, I left a comment in #15125.

I'm assuming the summary of this ticket should be "child's stdin to signal exit"?

comment:4 in reply to:  3 Changed 6 months ago by dcf

Summary: Close child's stdout to signal exit in meek-client-torbrowserClose child's stdin to signal exit in meek-client-torbrowser

Replying to cohosh:

I'm assuming the summary of this ticket should be "child's stdin to signal exit"?

That's right.

comment:5 Changed 6 months ago by dcf

Resolution: fixed
Status: merge_readyclosed

Merged in 0.32.

Note: See TracTickets for help on using tickets.