Browse Source

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 <cpuguy83@gmail.com>
Brian Goff 6 years ago
parent
commit
05390c4f6e

+ 2 - 2
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),
 	}
 }

+ 4 - 4
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),

+ 0 - 3
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

+ 3 - 3
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),
 	}

+ 0 - 33
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()),
-	}
-}

+ 1 - 1
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),
 	}

+ 4 - 4
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),
 	}

+ 2 - 2
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),

+ 2 - 2
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),
 	}
 

+ 1 - 1
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),
 	}

+ 2 - 1
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)