From 27b99639598ccf763d72a8b7b389e75470bd2251 Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Mon, 24 Aug 2015 10:42:28 -0700 Subject: [PATCH] client: always cancel in-flight request when do request This fits the way for go1.5 to cancel request. --- client/cancelreq.go | 20 ++++++++++++++ client/cancelreq_go14.go | 17 ++++++++++++ client/client.go | 4 ++- client/client_test.go | 19 -------------- client/fake_transport_go14_test.go | 41 +++++++++++++++++++++++++++++ client/fake_transport_test.go | 42 ++++++++++++++++++++++++++++++ 6 files changed, 123 insertions(+), 20 deletions(-) create mode 100644 client/cancelreq.go create mode 100644 client/cancelreq_go14.go create mode 100644 client/fake_transport_go14_test.go create mode 100644 client/fake_transport_test.go diff --git a/client/cancelreq.go b/client/cancelreq.go new file mode 100644 index 000000000..fefdb40e4 --- /dev/null +++ b/client/cancelreq.go @@ -0,0 +1,20 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// borrowed from golang/net/context/ctxhttp/cancelreq.go + +// +build go1.5 + +package client + +import "net/http" + +func requestCanceler(tr CancelableTransport, req *http.Request) func() { + ch := make(chan struct{}) + req.Cancel = ch + + return func() { + close(ch) + } +} diff --git a/client/cancelreq_go14.go b/client/cancelreq_go14.go new file mode 100644 index 000000000..2bed38a41 --- /dev/null +++ b/client/cancelreq_go14.go @@ -0,0 +1,17 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// borrowed from golang/net/context/ctxhttp/cancelreq_go14.go + +// +build !go1.5 + +package client + +import "net/http" + +func requestCanceler(tr CancelableTransport, req *http.Request) func() { + return func() { + tr.CancelRequest(req) + } +} diff --git a/client/client.go b/client/client.go index b687e4fc7..da86a0bc7 100644 --- a/client/client.go +++ b/client/client.go @@ -384,6 +384,8 @@ func (c *simpleHTTPClient) Do(ctx context.Context, act httpAction) (*http.Respon } defer hcancel() + reqcancel := requestCanceler(c.transport, req) + rtchan := make(chan roundTripResponse, 1) go func() { resp, err := c.transport.RoundTrip(req) @@ -399,7 +401,7 @@ func (c *simpleHTTPClient) Do(ctx context.Context, act httpAction) (*http.Respon resp, err = rtresp.resp, rtresp.err case <-hctx.Done(): // cancel and wait for request to actually exit before continuing - c.transport.CancelRequest(req) + reqcancel() rtresp := <-rtchan resp = rtresp.resp switch { diff --git a/client/client_test.go b/client/client_test.go index 32920ede1..be526b17b 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -109,25 +109,6 @@ func newFakeTransport() *fakeTransport { } } -func (t *fakeTransport) RoundTrip(*http.Request) (*http.Response, error) { - select { - case resp := <-t.respchan: - return resp, nil - case err := <-t.errchan: - return nil, err - case <-t.startCancel: - select { - // this simulates that the request is finished before cancel effects - case resp := <-t.respchan: - return resp, nil - // wait on finishCancel to simulate taking some amount of - // time while calling CancelRequest - case <-t.finishCancel: - return nil, errors.New("cancelled") - } - } -} - func (t *fakeTransport) CancelRequest(*http.Request) { t.startCancel <- struct{}{} } diff --git a/client/fake_transport_go14_test.go b/client/fake_transport_go14_test.go new file mode 100644 index 000000000..4a99a7d37 --- /dev/null +++ b/client/fake_transport_go14_test.go @@ -0,0 +1,41 @@ +// Copyright 2015 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !go1.5 + +package client + +import ( + "errors" + "net/http" +) + +func (t *fakeTransport) RoundTrip(req *http.Request) (*http.Response, error) { + select { + case resp := <-t.respchan: + return resp, nil + case err := <-t.errchan: + return nil, err + case <-t.startCancel: + select { + // this simulates that the request is finished before cancel effects + case resp := <-t.respchan: + return resp, nil + // wait on finishCancel to simulate taking some amount of + // time while calling CancelRequest + case <-t.finishCancel: + return nil, errors.New("cancelled") + } + } +} diff --git a/client/fake_transport_test.go b/client/fake_transport_test.go new file mode 100644 index 000000000..06761e266 --- /dev/null +++ b/client/fake_transport_test.go @@ -0,0 +1,42 @@ +// Copyright 2015 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build go1.5 + +package client + +import ( + "errors" + "net/http" +) + +func (t *fakeTransport) RoundTrip(req *http.Request) (*http.Response, error) { + select { + case resp := <-t.respchan: + return resp, nil + case err := <-t.errchan: + return nil, err + case <-t.startCancel: + case <-req.Cancel: + } + select { + // this simulates that the request is finished before cancel effects + case resp := <-t.respchan: + return resp, nil + // wait on finishCancel to simulate taking some amount of + // time while calling CancelRequest + case <-t.finishCancel: + return nil, errors.New("cancelled") + } +}