Reuse HTTP Clients (#1260)

cs3org · Oct 20, 2020 · 86b2550 · 86b2550
1 parent e221f0b
commit 86b2550
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 48 deletions.
diff --git a/changelog/unreleased/ocdav-fix-file-descriptor-leak.md b/changelog/unreleased/ocdav-fix-file-descriptor-leak.md
@@ -0,0 +1,5 @@
+Bugfix: Fix file descriptor leak on ocdav put handler 
+
+File descriptors on the ocdav service, especially on the put handler was leaking http connections. This PR addresses this.
+
+https://github.com/cs3org/reva/pull/1260
diff --git a/internal/http/services/datagateway/datagateway.go b/internal/http/services/datagateway/datagateway.go
@@ -69,6 +69,7 @@ func (c *config) init() {
 type svc struct {
 	conf    *config
 	handler http.Handler
+	client  *http.Client
 }
 
 // New returns a new datagateway
@@ -80,7 +81,13 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error)
 
 	conf.init()
 
-	s := &svc{conf: conf}
+	s := &svc{
+		conf: conf,
+		client: rhttp.GetHTTPClient(
+			rhttp.Timeout(time.Duration(conf.Timeout*int64(time.Second))),
+			rhttp.Insecure(conf.Insecure),
+		),
+	}
 	s.setHandler()
 	return s, nil
 }
@@ -170,11 +177,7 @@ func (s *svc) doHead(w http.ResponseWriter, r *http.Request) {
 
 	log.Debug().Str("target", claims.Target).Msg("sending request to internal data server")
 
-	httpClient := rhttp.GetHTTPClient(
-		rhttp.Context(ctx),
-		rhttp.Timeout(time.Duration(s.conf.Timeout*int64(time.Second))),
-		rhttp.Insecure(s.conf.Insecure),
-	)
+	httpClient := s.client
 	httpReq, err := rhttp.NewRequest(ctx, "HEAD", claims.Target, nil)
 	if err != nil {
 		log.Err(err).Msg("wrong request")
@@ -215,11 +218,7 @@ func (s *svc) doGet(w http.ResponseWriter, r *http.Request) {
 
 	log.Debug().Str("target", claims.Target).Msg("sending request to internal data server")
 
-	httpClient := rhttp.GetHTTPClient(
-		rhttp.Context(ctx),
-		rhttp.Timeout(time.Duration(s.conf.Timeout*int64(time.Second))),
-		rhttp.Insecure(s.conf.Insecure),
-	)
+	httpClient := s.client
 	httpReq, err := rhttp.NewRequest(ctx, "GET", claims.Target, nil)
 	if err != nil {
 		log.Err(err).Msg("wrong request")
@@ -275,11 +274,7 @@ func (s *svc) doPut(w http.ResponseWriter, r *http.Request) {
 
 	log.Debug().Str("target", claims.Target).Msg("sending request to internal data server")
 
-	httpClient := rhttp.GetHTTPClient(
-		rhttp.Context(ctx),
-		rhttp.Timeout(time.Duration(s.conf.Timeout*int64(time.Second))),
-		rhttp.Insecure(s.conf.Insecure),
-	)
+	httpClient := s.client
 	httpReq, err := rhttp.NewRequest(ctx, "PUT", target, r.Body)
 	if err != nil {
 		log.Err(err).Msg("wrong request")
@@ -336,11 +331,7 @@ func (s *svc) doPatch(w http.ResponseWriter, r *http.Request) {
 
 	log.Debug().Str("target", claims.Target).Msg("sending request to internal data server")
 
-	httpClient := rhttp.GetHTTPClient(
-		rhttp.Context(ctx),
-		rhttp.Timeout(time.Duration(s.conf.Timeout*int64(time.Second))),
-		rhttp.Insecure(s.conf.Insecure),
-	)
+	httpClient := s.client
 	httpReq, err := rhttp.NewRequest(ctx, "PATCH", target, r.Body)
 	if err != nil {
 		log.Err(err).Msg("wrong request")

diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go
@@ -25,7 +25,6 @@ import (
 	"net/http"
 	"path"
 	"strings"
-	"time"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
@@ -282,11 +281,7 @@ func (s *svc) descend(ctx context.Context, client gateway.GatewayAPIClient, src
 		}
 		httpDownloadReq.Header.Set(datagateway.TokenTransportHeader, dRes.Token)
 
-		httpDownloadClient := rhttp.GetHTTPClient(
-			rhttp.Context(ctx),
-			rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
-			rhttp.Insecure(s.c.Insecure),
-		)
+		httpDownloadClient := s.client
 
 		httpDownloadRes, err := httpDownloadClient.Do(httpDownloadReq)
 		if err != nil {
@@ -314,11 +309,7 @@ func (s *svc) tusUpload(ctx context.Context, dataServerURL string, transferToken
 	// create the tus client.
 	c := tus.DefaultConfig()
 	c.Resume = true
-	c.HttpClient = rhttp.GetHTTPClient(
-		rhttp.Context(ctx),
-		rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
-		rhttp.Insecure(s.c.Insecure),
-	)
+	c.HttpClient = s.client
 	c.Store, err = memorystore.NewMemoryStore()
 	if err != nil {
 		return err

diff --git a/internal/http/services/owncloud/ocdav/get.go b/internal/http/services/owncloud/ocdav/get.go
@@ -121,11 +121,7 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) {
 		return
 	}
 	httpReq.Header.Set(datagateway.TokenTransportHeader, dRes.Token)
-	httpClient := rhttp.GetHTTPClient(
-		rhttp.Context(ctx),
-		rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
-		rhttp.Insecure(s.c.Insecure),
-	)
+	httpClient := s.client
 
 	httpRes, err := httpClient.Do(httpReq)
 	if err != nil {

diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go
@@ -27,11 +27,13 @@ import (
 	"os"
 	"path"
 	"strings"
+	"time"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
+	"github.com/cs3org/reva/pkg/rhttp"
 	"github.com/cs3org/reva/pkg/rhttp/global"
 	"github.com/cs3org/reva/pkg/rhttp/router"
 	"github.com/cs3org/reva/pkg/sharedconf"
@@ -87,6 +89,7 @@ type svc struct {
 	c             *Config
 	webDavHandler *WebDavHandler
 	davHandler    *DavHandler
+	client        *http.Client
 }
 
 // New returns a new ocdav
@@ -106,6 +109,10 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error)
 		c:             conf,
 		webDavHandler: new(WebDavHandler),
 		davHandler:    new(DavHandler),
+		client: rhttp.GetHTTPClient(
+			rhttp.Timeout(time.Duration(conf.Timeout*int64(time.Second))),
+			rhttp.Insecure(conf.Insecure),
+		),
 	}
 	// initialize handlers and set default configs
 	if err := s.webDavHandler.init(conf.WebdavNamespace); err != nil {

diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go
@@ -31,7 +31,6 @@ import (
 	"github.com/cs3org/reva/internal/http/services/datagateway"
 	"github.com/cs3org/reva/internal/http/utils"
 	"github.com/cs3org/reva/pkg/appctx"
-	"github.com/cs3org/reva/pkg/rhttp"
 	tokenpkg "github.com/cs3org/reva/pkg/token"
 	"github.com/eventials/go-tus"
 	"github.com/eventials/go-tus/memorystore"
@@ -272,11 +271,8 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) {
 	// create the tus client.
 	c := tus.DefaultConfig()
 	c.Resume = true
-	c.HttpClient = rhttp.GetHTTPClient(
-		rhttp.Context(ctx),
-		rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
-		rhttp.Insecure(s.c.Insecure),
-	)
+	c.HttpClient = s.client
+
 	c.Store, err = memorystore.NewMemoryStore()
 	if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)

diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go
@@ -193,11 +193,7 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) {
 		var httpRes *http.Response
 
 		if length != 0 {
-			httpClient := rhttp.GetHTTPClient(
-				rhttp.Context(ctx),
-				rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
-				rhttp.Insecure(s.c.Insecure),
-			)
+			httpClient := s.client
 			httpReq, err := rhttp.NewRequest(ctx, "PATCH", uRes.UploadEndpoint, r.Body)
 			if err != nil {
 				log.Err(err).Msg("wrong request")