From 05390c4f6e9e2761d56ab374b55defdf3b09a214 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 27 Nov 2018 14:13:43 -0800 Subject: [PATCH] Rely on request.Context() cancellation The cancellable handler is no longer needed as the context that is passed with the http request will be cancelled just like the close notifier was doing. Signed-off-by: Brian Goff --- api/server/router/build/build.go | 4 +-- api/server/router/container/container.go | 8 ++--- .../router/container/container_routes.go | 3 -- api/server/router/image/image.go | 6 ++-- api/server/router/local.go | 33 ------------------- api/server/router/network/network.go | 2 +- api/server/router/plugin/plugin.go | 8 ++--- api/server/router/swarm/cluster.go | 4 +-- api/server/router/system/system.go | 4 +-- api/server/router/volume/volume.go | 2 +- api/server/server.go | 3 +- 11 files changed, 21 insertions(+), 56 deletions(-) diff --git a/api/server/router/build/build.go b/api/server/router/build/build.go index e6b42ad163..8ad89ac2b7 100644 --- a/api/server/router/build/build.go +++ b/api/server/router/build/build.go @@ -31,8 +31,8 @@ func (r *buildRouter) Routes() []router.Route { func (r *buildRouter) initRoutes() { r.routes = []router.Route{ - router.NewPostRoute("/build", r.postBuild, router.WithCancel), - router.NewPostRoute("/build/prune", r.postPrune, router.WithCancel), + router.NewPostRoute("/build", r.postBuild), + router.NewPostRoute("/build/prune", r.postPrune), router.NewPostRoute("/build/cancel", r.postCancel), } } diff --git a/api/server/router/container/container.go b/api/server/router/container/container.go index 358f2bc2c1..401be4a3fb 100644 --- a/api/server/router/container/container.go +++ b/api/server/router/container/container.go @@ -38,8 +38,8 @@ func (r *containerRouter) initRoutes() { router.NewGetRoute("/containers/{name:.*}/changes", r.getContainersChanges), router.NewGetRoute("/containers/{name:.*}/json", r.getContainersByName), router.NewGetRoute("/containers/{name:.*}/top", r.getContainersTop), - router.NewGetRoute("/containers/{name:.*}/logs", r.getContainersLogs, router.WithCancel), - router.NewGetRoute("/containers/{name:.*}/stats", r.getContainersStats, router.WithCancel), + router.NewGetRoute("/containers/{name:.*}/logs", r.getContainersLogs), + router.NewGetRoute("/containers/{name:.*}/stats", r.getContainersStats), router.NewGetRoute("/containers/{name:.*}/attach/ws", r.wsContainersAttach), router.NewGetRoute("/exec/{id:.*}/json", r.getExecByID), router.NewGetRoute("/containers/{name:.*}/archive", r.getContainersArchive), @@ -51,7 +51,7 @@ func (r *containerRouter) initRoutes() { router.NewPostRoute("/containers/{name:.*}/restart", r.postContainersRestart), router.NewPostRoute("/containers/{name:.*}/start", r.postContainersStart), router.NewPostRoute("/containers/{name:.*}/stop", r.postContainersStop), - router.NewPostRoute("/containers/{name:.*}/wait", r.postContainersWait, router.WithCancel), + router.NewPostRoute("/containers/{name:.*}/wait", r.postContainersWait), router.NewPostRoute("/containers/{name:.*}/resize", r.postContainersResize), router.NewPostRoute("/containers/{name:.*}/attach", r.postContainersAttach), router.NewPostRoute("/containers/{name:.*}/copy", r.postContainersCopy), // Deprecated since 1.8, Errors out since 1.12 @@ -60,7 +60,7 @@ func (r *containerRouter) initRoutes() { router.NewPostRoute("/exec/{name:.*}/resize", r.postContainerExecResize), router.NewPostRoute("/containers/{name:.*}/rename", r.postContainerRename), router.NewPostRoute("/containers/{name:.*}/update", r.postContainerUpdate), - router.NewPostRoute("/containers/prune", r.postContainersPrune, router.WithCancel), + router.NewPostRoute("/containers/prune", r.postContainersPrune), router.NewPostRoute("/commit", r.postCommit), // PUT router.NewPutRoute("/containers/{name:.*}/archive", r.putContainersArchive), diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index c1f32724ee..9000128ea0 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -338,9 +338,6 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons } } - // Note: the context should get canceled if the client closes the - // connection since this handler has been wrapped by the - // router.WithCancel() wrapper. waitC, err := s.backend.ContainerWait(ctx, vars["name"], waitCondition) if err != nil { return err diff --git a/api/server/router/image/image.go b/api/server/router/image/image.go index 6d5d87f63c..42f89153b5 100644 --- a/api/server/router/image/image.go +++ b/api/server/router/image/image.go @@ -34,10 +34,10 @@ func (r *imageRouter) initRoutes() { router.NewGetRoute("/images/{name:.*}/json", r.getImagesByName), // POST router.NewPostRoute("/images/load", r.postImagesLoad), - router.NewPostRoute("/images/create", r.postImagesCreate, router.WithCancel), - router.NewPostRoute("/images/{name:.*}/push", r.postImagesPush, router.WithCancel), + router.NewPostRoute("/images/create", r.postImagesCreate), + router.NewPostRoute("/images/{name:.*}/push", r.postImagesPush), router.NewPostRoute("/images/{name:.*}/tag", r.postImagesTag), - router.NewPostRoute("/images/prune", r.postImagesPrune, router.WithCancel), + router.NewPostRoute("/images/prune", r.postImagesPrune), // DELETE router.NewDeleteRoute("/images/{name:.*}", r.deleteImages), } diff --git a/api/server/router/local.go b/api/server/router/local.go index 79a323928e..3f629e8e4e 100644 --- a/api/server/router/local.go +++ b/api/server/router/local.go @@ -1,9 +1,6 @@ package router // import "github.com/docker/docker/api/server/router" import ( - "context" - "net/http" - "github.com/docker/docker/api/server/httputils" ) @@ -72,33 +69,3 @@ func NewOptionsRoute(path string, handler httputils.APIFunc, opts ...RouteWrappe func NewHeadRoute(path string, handler httputils.APIFunc, opts ...RouteWrapper) Route { return NewRoute("HEAD", path, handler, opts...) } - -func cancellableHandler(h httputils.APIFunc) httputils.APIFunc { - return func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - if notifier, ok := w.(http.CloseNotifier); ok { - notify := notifier.CloseNotify() - notifyCtx, cancel := context.WithCancel(ctx) - finished := make(chan struct{}) - defer close(finished) - ctx = notifyCtx - go func() { - select { - case <-notify: - cancel() - case <-finished: - } - }() - } - return h(ctx, w, r, vars) - } -} - -// WithCancel makes new route which embeds http.CloseNotifier feature to -// context.Context of handler. -func WithCancel(r Route) Route { - return localRoute{ - method: r.Method(), - path: r.Path(), - handler: cancellableHandler(r.Handler()), - } -} diff --git a/api/server/router/network/network.go b/api/server/router/network/network.go index 4eee970793..d31883500b 100644 --- a/api/server/router/network/network.go +++ b/api/server/router/network/network.go @@ -36,7 +36,7 @@ func (r *networkRouter) initRoutes() { router.NewPostRoute("/networks/create", r.postNetworkCreate), router.NewPostRoute("/networks/{id:.*}/connect", r.postNetworkConnect), router.NewPostRoute("/networks/{id:.*}/disconnect", r.postNetworkDisconnect), - router.NewPostRoute("/networks/prune", r.postNetworksPrune, router.WithCancel), + router.NewPostRoute("/networks/prune", r.postNetworksPrune), // DELETE router.NewDeleteRoute("/networks/{id:.*}", r.deleteNetwork), } diff --git a/api/server/router/plugin/plugin.go b/api/server/router/plugin/plugin.go index 7a4f987aa3..96190b3d03 100644 --- a/api/server/router/plugin/plugin.go +++ b/api/server/router/plugin/plugin.go @@ -28,11 +28,11 @@ func (r *pluginRouter) initRoutes() { router.NewGetRoute("/plugins/{name:.*}/json", r.inspectPlugin), router.NewGetRoute("/plugins/privileges", r.getPrivileges), router.NewDeleteRoute("/plugins/{name:.*}", r.removePlugin), - router.NewPostRoute("/plugins/{name:.*}/enable", r.enablePlugin), // PATCH? + router.NewPostRoute("/plugins/{name:.*}/enable", r.enablePlugin), router.NewPostRoute("/plugins/{name:.*}/disable", r.disablePlugin), - router.NewPostRoute("/plugins/pull", r.pullPlugin, router.WithCancel), - router.NewPostRoute("/plugins/{name:.*}/push", r.pushPlugin, router.WithCancel), - router.NewPostRoute("/plugins/{name:.*}/upgrade", r.upgradePlugin, router.WithCancel), + router.NewPostRoute("/plugins/pull", r.pullPlugin), + router.NewPostRoute("/plugins/{name:.*}/push", r.pushPlugin), + router.NewPostRoute("/plugins/{name:.*}/upgrade", r.upgradePlugin), router.NewPostRoute("/plugins/{name:.*}/set", r.setPlugin), router.NewPostRoute("/plugins/create", r.createPlugin), } diff --git a/api/server/router/swarm/cluster.go b/api/server/router/swarm/cluster.go index 52f950a3a9..96385d619e 100644 --- a/api/server/router/swarm/cluster.go +++ b/api/server/router/swarm/cluster.go @@ -37,7 +37,7 @@ func (sr *swarmRouter) initRoutes() { router.NewPostRoute("/services/create", sr.createService), router.NewPostRoute("/services/{id}/update", sr.updateService), router.NewDeleteRoute("/services/{id}", sr.removeService), - router.NewGetRoute("/services/{id}/logs", sr.getServiceLogs, router.WithCancel), + router.NewGetRoute("/services/{id}/logs", sr.getServiceLogs), router.NewGetRoute("/nodes", sr.getNodes), router.NewGetRoute("/nodes/{id}", sr.getNode), @@ -46,7 +46,7 @@ func (sr *swarmRouter) initRoutes() { router.NewGetRoute("/tasks", sr.getTasks), router.NewGetRoute("/tasks/{id}", sr.getTask), - router.NewGetRoute("/tasks/{id}/logs", sr.getTaskLogs, router.WithCancel), + router.NewGetRoute("/tasks/{id}/logs", sr.getTaskLogs), router.NewGetRoute("/secrets", sr.getSecrets), router.NewPostRoute("/secrets/create", sr.createSecret), diff --git a/api/server/router/system/system.go b/api/server/router/system/system.go index bbe32fb4bb..e0c4a3eefb 100644 --- a/api/server/router/system/system.go +++ b/api/server/router/system/system.go @@ -30,10 +30,10 @@ func NewRouter(b Backend, c ClusterBackend, fscache *fscache.FSCache, builder *b r.routes = []router.Route{ router.NewOptionsRoute("/{anyroute:.*}", optionsHandler), router.NewGetRoute("/_ping", r.pingHandler), - router.NewGetRoute("/events", r.getEvents, router.WithCancel), + router.NewGetRoute("/events", r.getEvents), router.NewGetRoute("/info", r.getInfo), router.NewGetRoute("/version", r.getVersion), - router.NewGetRoute("/system/df", r.getDiskUsage, router.WithCancel), + router.NewGetRoute("/system/df", r.getDiskUsage), router.NewPostRoute("/auth", r.postAuth), } diff --git a/api/server/router/volume/volume.go b/api/server/router/volume/volume.go index 04f365e370..875497b3eb 100644 --- a/api/server/router/volume/volume.go +++ b/api/server/router/volume/volume.go @@ -29,7 +29,7 @@ func (r *volumeRouter) initRoutes() { router.NewGetRoute("/volumes/{name:.*}", r.getVolumeByName), // POST router.NewPostRoute("/volumes/create", r.postVolumesCreate), - router.NewPostRoute("/volumes/prune", r.postVolumesPrune, router.WithCancel), + router.NewPostRoute("/volumes/prune", r.postVolumesPrune), // DELETE router.NewDeleteRoute("/volumes/{name:.*}", r.deleteVolumes), } diff --git a/api/server/server.go b/api/server/server.go index 815b2b12c0..9c9b8fec6a 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -129,7 +129,8 @@ func (s *Server) makeHTTPHandler(handler httputils.APIFunc) http.HandlerFunc { // use intermediate variable to prevent "should not use basic type // string as key in context.WithValue" golint errors - ctx := context.WithValue(context.Background(), dockerversion.UAStringKey{}, r.Header.Get("User-Agent")) + ctx := context.WithValue(r.Context(), dockerversion.UAStringKey{}, r.Header.Get("User-Agent")) + r = r.WithContext(ctx) handlerFunc := s.handlerWithGlobalMiddlewares(handler) vars := mux.Vars(r)