Commit 1722ec22 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

cgi: close stdout reader pipe when finished

This causes the child, if still writing, to get an error or
SIGPIPE and most likely exit so our subsequent wait can
finish.

A more guaranteed fix would be putting a time limit on the
child's overall execution, but this fixes the problem
I was having.

Fixes #2059

R=rsc
CC=golang-dev
https://golang.org/cl/4675081
parent d53385fd
...@@ -338,7 +338,8 @@ func (c *Cmd) StdinPipe() (io.WriteCloser, os.Error) { ...@@ -338,7 +338,8 @@ func (c *Cmd) StdinPipe() (io.WriteCloser, os.Error) {
// StdoutPipe returns a pipe that will be connected to the command's // StdoutPipe returns a pipe that will be connected to the command's
// standard output when the command starts. // standard output when the command starts.
func (c *Cmd) StdoutPipe() (io.Reader, os.Error) { // The pipe will be closed automatically after Wait sees the command exit.
func (c *Cmd) StdoutPipe() (io.ReadCloser, os.Error) {
if c.Stdout != nil { if c.Stdout != nil {
return nil, os.NewError("exec: Stdout already set") return nil, os.NewError("exec: Stdout already set")
} }
...@@ -357,7 +358,8 @@ func (c *Cmd) StdoutPipe() (io.Reader, os.Error) { ...@@ -357,7 +358,8 @@ func (c *Cmd) StdoutPipe() (io.Reader, os.Error) {
// StderrPipe returns a pipe that will be connected to the command's // StderrPipe returns a pipe that will be connected to the command's
// standard error when the command starts. // standard error when the command starts.
func (c *Cmd) StderrPipe() (io.Reader, os.Error) { // The pipe will be closed automatically after Wait sees the command exit.
func (c *Cmd) StderrPipe() (io.ReadCloser, os.Error) {
if c.Stderr != nil { if c.Stderr != nil {
return nil, os.NewError("exec: Stderr already set") return nil, os.NewError("exec: Stderr already set")
} }
......
...@@ -187,6 +187,7 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { ...@@ -187,6 +187,7 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
return return
} }
defer cmd.Wait() defer cmd.Wait()
defer stdoutRead.Close()
linebody, _ := bufio.NewReaderSize(stdoutRead, 1024) linebody, _ := bufio.NewReaderSize(stdoutRead, 1024)
headers := make(http.Header) headers := make(http.Header)
......
...@@ -12,10 +12,14 @@ import ( ...@@ -12,10 +12,14 @@ import (
"fmt" "fmt"
"http" "http"
"http/httptest" "http/httptest"
"io"
"os" "os"
"net"
"path/filepath" "path/filepath"
"strconv"
"strings" "strings"
"testing" "testing"
"time"
"runtime" "runtime"
) )
...@@ -304,8 +308,76 @@ func TestInternalRedirect(t *testing.T) { ...@@ -304,8 +308,76 @@ func TestInternalRedirect(t *testing.T) {
runCgiTest(t, h, "GET /test.cgi?loc=/foo HTTP/1.0\nHost: example.com\n\n", expectedMap) runCgiTest(t, h, "GET /test.cgi?loc=/foo HTTP/1.0\nHost: example.com\n\n", expectedMap)
} }
// TestCopyError tests that we kill the process if there's an error copying
// its output. (for example, from the client having gone away)
func TestCopyError(t *testing.T) {
if skipTest(t) || runtime.GOOS == "windows" {
return
}
h := &Handler{
Path: "testdata/test.cgi",
Root: "/test.cgi",
}
ts := httptest.NewServer(h)
defer ts.Close()
conn, err := net.Dial("tcp", ts.Listener.Addr().String())
if err != nil {
t.Fatal(err)
}
req, _ := http.NewRequest("GET", "http://example.com/test.cgi?bigresponse=1", nil)
err = req.Write(conn)
if err != nil {
t.Fatalf("Write: %v", err)
}
res, err := http.ReadResponse(bufio.NewReader(conn), req)
if err != nil {
t.Fatalf("ReadResponse: %v", err)
}
pidstr := res.Header.Get("X-CGI-Pid")
if pidstr == "" {
t.Fatalf("expected an X-CGI-Pid header in response")
}
pid, err := strconv.Atoi(pidstr)
if err != nil {
t.Fatalf("invalid X-CGI-Pid value")
}
var buf [5000]byte
n, err := io.ReadFull(res.Body, buf[:])
if err != nil {
t.Fatalf("ReadFull: %d bytes, %v", n, err)
}
childRunning := func() bool {
p, err := os.FindProcess(pid)
if err != nil {
return false
}
return p.Signal(os.UnixSignal(0)) == nil
}
if !childRunning() {
t.Fatalf("pre-conn.Close, expected child to be running")
}
conn.Close()
if tries := 0; childRunning() {
for tries < 5 && childRunning() {
time.Sleep(50e6 * int64(tries))
tries++
}
if childRunning() {
t.Fatalf("post-conn.Close, expected child to be gone")
}
}
}
func TestDirUnix(t *testing.T) { func TestDirUnix(t *testing.T) {
if runtime.GOOS == "windows" { if skipTest(t) || runtime.GOOS == "windows" {
return return
} }
...@@ -333,7 +405,7 @@ func TestDirUnix(t *testing.T) { ...@@ -333,7 +405,7 @@ func TestDirUnix(t *testing.T) {
} }
func TestDirWindows(t *testing.T) { func TestDirWindows(t *testing.T) {
if runtime.GOOS != "windows" { if skipTest(t) || runtime.GOOS != "windows" {
return return
} }
......
...@@ -25,9 +25,17 @@ my $p = sub { ...@@ -25,9 +25,17 @@ my $p = sub {
# With carriage returns # With carriage returns
$p->("Content-Type: text/html"); $p->("Content-Type: text/html");
$p->("X-CGI-Pid: $$");
$p->("X-Test-Header: X-Test-Value"); $p->("X-Test-Header: X-Test-Value");
$p->(""); $p->("");
if ($params->{"bigresponse"}) {
for (1..1024) {
print "A" x 1024, "\n";
}
exit 0;
}
print "test=Hello CGI\n"; print "test=Hello CGI\n";
foreach my $k (sort keys %$params) { foreach my $k (sort keys %$params) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment