diff --git a/probe/appclient/app_client.go b/probe/appclient/app_client.go index 84a47fec6..bffaf0d5c 100644 --- a/probe/appclient/app_client.go +++ b/probe/appclient/app_client.go @@ -19,9 +19,10 @@ import ( ) const ( - initialBackoff = 1 * time.Second - maxBackoff = 60 * time.Second - dialTimeout = 5 * time.Second + httpClientTimeout = 4 * time.Second + initialBackoff = 1 * time.Second + maxBackoff = 60 * time.Second + dialTimeout = 5 * time.Second ) // AppClient is a client to an app for dealing with controls. @@ -76,6 +77,7 @@ func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlH target: target, client: http.Client{ Transport: httpTransport, + Timeout: httpClientTimeout, }, wsDialer: websocket.Dialer{ TLSClientConfig: httpTransport.TLSClientConfig, @@ -245,6 +247,10 @@ func (c *appClient) publish(r io.Reader) error { req.Header.Set("Content-Encoding", "gzip") req.Header.Set("Content-Type", "application/msgpack") // req.Header.Set("Content-Type", "application/binary") // TODO: we should use http.DetectContentType(..) on the gob'ed + + // Make sure this request is cancelled when we stop the client + req.Cancel = c.quit + resp, err := c.client.Do(req) if err != nil { return err diff --git a/probe/appclient/app_client_internal_test.go b/probe/appclient/app_client_internal_test.go index 4508f054b..2a990e3a1 100644 --- a/probe/appclient/app_client_internal_test.go +++ b/probe/appclient/app_client_internal_test.go @@ -173,3 +173,59 @@ func TestAppClientDetails(t *testing.T) { return } } + +// Make sure Stopping a client works even if the connection or the remote app +// gets stuck for whatever reason. +// See https://github.com/weaveworks/scope/issues/1576 +func TestStop(t *testing.T) { + var ( + rpt = report.MakeReport() + stopHanging = make(chan struct{}) + receivedReport = make(chan struct{}) + ) + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + close(receivedReport) + <-stopHanging + }) + + s := httptest.NewServer(handlers.CompressHandler(handler)) + defer s.Close() + + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + pc := ProbeConfig{ + Token: "", + ProbeID: "", + Insecure: false, + } + + p, err := NewAppClient(pc, u.Host, s.URL, nil) + if err != nil { + t.Fatal(err) + } + + rp := NewReportPublisher(p) + + // Make sure the app received our report and is stuck + for done := false; !done; { + select { + case <-receivedReport: + done = true + default: + if err := rp.Publish(rpt); err != nil { + t.Error(err) + } + time.Sleep(10 * time.Millisecond) + } + } + + // Close the client while the app is stuck + p.Stop() + + // Let the server go so that the test can end + close(stopHanging) +}