From c2ea9d119df82705b7b37d88c9f57b1d6327de3b Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Fri, 23 Aug 2019 10:15:52 +0200 Subject: [PATCH] lib/connections: Upgrade QUIC package, use contexts for timeout (#5972) --- go.mod | 12 ++++---- go.sum | 35 ++++++++++++++-------- lib/connections/quic_dial.go | 34 +++++++++------------ lib/connections/quic_listen.go | 55 +++++++++++----------------------- 4 files changed, 61 insertions(+), 75 deletions(-) diff --git a/go.mod b/go.mod index e22b2e746..b5b4a8a56 100644 --- a/go.mod +++ b/go.mod @@ -15,16 +15,18 @@ require ( github.com/gobwas/glob v0.0.0-20170212200151-51eb1ee00b6d github.com/gogo/protobuf v1.2.1 github.com/golang/groupcache v0.0.0-20171101203131-84a468cf14b4 + github.com/golang/mock v1.3.1 // indirect + github.com/golang/protobuf v1.3.2 // indirect github.com/jackpal/gateway v0.0.0-20161225004348-5795ac81146e github.com/kballard/go-shellquote v0.0.0-20170619183022-cd60e84ee657 github.com/kr/pretty v0.1.0 // indirect github.com/lib/pq v1.2.0 - github.com/lucas-clemente/quic-go v0.11.2 + github.com/lucas-clemente/quic-go v0.12.0 github.com/maruel/panicparse v1.3.0 github.com/mattn/go-isatty v0.0.9 github.com/minio/sha256-simd v0.0.0-20190117184323-cc1980cb0338 - github.com/onsi/ginkgo v1.8.0 // indirect - github.com/onsi/gomega v1.5.0 // indirect + github.com/onsi/ginkgo v1.9.0 // indirect + github.com/onsi/gomega v1.6.0 // indirect github.com/oschwald/geoip2-golang v1.3.0 github.com/oschwald/maxminddb-golang v0.0.0-20170901134056-26fe5ace1c70 // indirect github.com/petermattis/goid v0.0.0-20170816195418-3db12ebb2a59 // indirect @@ -38,8 +40,8 @@ require ( github.com/thejerf/suture v3.0.2+incompatible github.com/urfave/cli v1.21.0 github.com/vitrun/qart v0.0.0-20160531060029-bf64b92db6b0 - golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8 - golang.org/x/net v0.0.0-20190613194153-d28f0bde5980 + golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586 + golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 golang.org/x/text v0.3.2 golang.org/x/time v0.0.0-20170927054726-6dc17368e09b gopkg.in/asn1-ber.v1 v1.0.0-20170511165959-379148ca0225 // indirect diff --git a/go.sum b/go.sum index a56950d63..cd5225a6f 100644 --- a/go.sum +++ b/go.sum @@ -50,10 +50,15 @@ github.com/golang/groupcache v0.0.0-20171101203131-84a468cf14b4 h1:6o8aP0LGMKzo3 github.com/golang/groupcache v0.0.0-20171101203131-84a468cf14b4/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/mock v1.2.0 h1:28o5sBqPkBsMGnC6b4MvE2TzSr5/AT4c/1fLqVGIwlk= github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= +github.com/golang/mock v1.3.1 h1:qGJ6qTW+x6xX/my+8YUVl4WNpX9B7+/l2tRsHGZ7f2s= +github.com/golang/mock v1.3.1/go.mod h1:sBzyDLLjw3U8JLTeZvSv8jJB+tU5PVekmnlKIyFUx0Y= github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/golang/protobuf v1.3.0/go.mod h1:Qd/q+1AKNOZr9uGQzbzCmRO6sUih6GTPZv6a1/R87v0= github.com/golang/protobuf v1.3.1 h1:YF8+flBXS5eO826T4nzqPrxfhQThhXl0YzfuUPu4SBg= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs= +github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/snappy v0.0.1 h1:Qgr9rKW7uDUkrbSmQeiDsGa8SjGyCOGtuasMWwvp2P4= github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= @@ -75,10 +80,11 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/lib/pq v1.2.0 h1:LXpIM/LZ5xGFhOpXAQUIMM1HdyqzVYM13zNdjCEEcA0= github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= -github.com/lucas-clemente/quic-go v0.11.2 h1:Mop0ac3zALaBR3wGs6j8OYe/tcFvFsxTUFMkE/7yUOI= -github.com/lucas-clemente/quic-go v0.11.2/go.mod h1:PpMmPfPKO9nKJ/psF49ESTAGQSdfXxlg1otPbEB2nOw= -github.com/marten-seemann/qtls v0.2.3 h1:0yWJ43C62LsZt08vuQJDK1uC1czUc3FJeCLPoNAI4vA= -github.com/marten-seemann/qtls v0.2.3/go.mod h1:xzjG7avBwGGbdZ8dTGxlBnLArsVKLvwmjgmPuiQEcYk= +github.com/lucas-clemente/quic-go v0.12.0 h1:dYHUyB50gEQlK3KqytmNySzuyzAcaQ3iuI2ZReAfVrE= +github.com/lucas-clemente/quic-go v0.12.0/go.mod h1:UXJJPE4RfFef/xPO5wQm0tITK8gNfqwTxjbE7s3Vb8s= +github.com/marten-seemann/qpack v0.1.0/go.mod h1:LFt1NU/Ptjip0C2CPkhimBz5CGE3WGDAUWqna+CNTrI= +github.com/marten-seemann/qtls v0.3.2 h1:O7awy4bHEzSX/K3h+fZig3/Vo03s/RxlxgsAk9sYamI= +github.com/marten-seemann/qtls v0.3.2/go.mod h1:xzjG7avBwGGbdZ8dTGxlBnLArsVKLvwmjgmPuiQEcYk= github.com/maruel/panicparse v1.3.0 h1:1Ep/RaYoSL1r5rTILHQQbyzHG8T4UP5ZbQTYTo4bdDc= github.com/maruel/panicparse v1.3.0/go.mod h1:vszMjr5QQ4F5FSRfraldcIA/BCw5xrdLL+zEcU2nRBs= github.com/mattn/go-colorable v0.1.1 h1:G1f5SKeVxmagw/IyvzvtZE4Gybcc4Tr1tf7I8z0XgOg= @@ -100,12 +106,12 @@ github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRW github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.7.0 h1:WSHQ+IS43OoUrWtD1/bbclrwK8TTH5hzp+umCiuxHgs= github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= -github.com/onsi/ginkgo v1.8.0 h1:VkHVNpR4iVnU8XQR6DBm8BqYjN7CRzw+xKUbVVbbW9w= -github.com/onsi/ginkgo v1.8.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= +github.com/onsi/ginkgo v1.9.0 h1:SZjF721BByVj8QH636/8S2DnX4n0Re3SteMmw3N+tzc= +github.com/onsi/ginkgo v1.9.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/gomega v1.4.3 h1:RE1xgDvH7imwFD45h+u2SgIfERHlS2yNG4DObb5BSKU= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= -github.com/onsi/gomega v1.5.0 h1:izbySO9zDPmjJ8rDjLvkA2zJHIo+HkYXHnf7eN7SSyo= -github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= +github.com/onsi/gomega v1.6.0 h1:8XTW0fcJZEq9q+Upcyws4JSGua2MFysCL5xkaSgHc+M= +github.com/onsi/gomega v1.6.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/oschwald/geoip2-golang v1.3.0 h1:D+Hsdos1NARPbzZ2aInUHZL+dApIzo8E0ErJVsWcku8= github.com/oschwald/geoip2-golang v1.3.0/go.mod h1:0LTTzix/Ao1uMvOhAV4iLU0Lz7eCrP94qZWBTDKf0iE= github.com/oschwald/maxminddb-golang v0.0.0-20170901134056-26fe5ace1c70 h1:XGLYUmodtNzThosQ8GkMvj9TiIB/uWsP8NfxKSa3aDc= @@ -159,17 +165,20 @@ golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnf golang.org/x/crypto v0.0.0-20190228161510-8dd112bcdc25/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8 h1:1wopBVtVdWnn03fZelqdXTqk7U7zPQCb+T4rbU9ZEoU= -golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586 h1:7KByu05hhLed2MO29w7p1XfZvZ13m8mub3shuVftRs0= +golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/net v0.0.0-20190228165749-92fc7df08ae7/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190613194153-d28f0bde5980 h1:dfGZHvZk057jK2MCeWus/TowKpJ8y4AmooUzdBSR9GU= -golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 h1:fHDIZ2oxGnUZRN6WgWFCbYBjH9uqVPRCUVUDhs0wnbA= +golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f h1:Bl/8QSvNqXvPGPGXa2z5xUTmV7VDcZyvRZ+QQXkXTZQ= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180926160741-c2ed4eda69e7/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -191,6 +200,8 @@ golang.org/x/time v0.0.0-20170927054726-6dc17368e09b h1:3X+R0qq1+64izd8es+EttB6q golang.org/x/time v0.0.0-20170927054726-6dc17368e09b/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= +google.golang.org/genproto v0.0.0-20180831171423-11092d34479b/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/asn1-ber.v1 v1.0.0-20170511165959-379148ca0225 h1:JBwmEvLfCqgPcIq8MjVMQxsF3LVL4XG/HH0qiG0+IFY= gopkg.in/asn1-ber.v1 v1.0.0-20170511165959-379148ca0225/go.mod h1:cuepJuh7vyXfUyUwEgHQXw849cJrilpS5NeIjOWESAw= diff --git a/lib/connections/quic_dial.go b/lib/connections/quic_dial.go index 62aba5c0f..cea2e54b3 100644 --- a/lib/connections/quic_dial.go +++ b/lib/connections/quic_dial.go @@ -11,6 +11,7 @@ package connections import ( "context" "crypto/tls" + "fmt" "net" "net/url" "time" @@ -22,7 +23,13 @@ import ( "github.com/syncthing/syncthing/lib/protocol" ) -const quicPriority = 100 +const ( + quicPriority = 100 + + // The timeout for connecting, accepting and creating the various + // streams. + quicOperationTimeout = 10 * time.Second +) func init() { factory := &quicDialerFactory{} @@ -60,38 +67,25 @@ func (d *quicDialer) Dial(_ protocol.DeviceID, uri *url.URL) (internalConn, erro } } - ctx, _ := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), quicOperationTimeout) + defer cancel() + session, err := quic.DialContext(ctx, conn, addr, uri.Host, d.tlsCfg, quicConfig) if err != nil { if createdConn != nil { _ = createdConn.Close() } - return internalConn{}, err + return internalConn{}, fmt.Errorf("dial: %v", err) } - // OpenStreamSync is blocks, but we want to make sure the connection is usable - // before we start killing off other connections, so do the dance. - ok := make(chan struct{}) - go func() { - select { - case <-ok: - return - case <-time.After(10 * time.Second): - l.Debugln("timed out waiting for OpenStream on", session.RemoteAddr()) - // This will unblock OpenStreamSync - _ = session.Close() - } - }() - - stream, err := session.OpenStreamSync() - close(ok) + stream, err := session.OpenStreamSync(ctx) if err != nil { // It's ok to close these, this does not close the underlying packetConn. _ = session.Close() if createdConn != nil { _ = createdConn.Close() } - return internalConn{}, err + return internalConn{}, fmt.Errorf("open stream: %v", err) } return internalConn{&quicTlsConn{session, stream, createdConn}, connTypeQUICClient, quicPriority}, nil diff --git a/lib/connections/quic_listen.go b/lib/connections/quic_listen.go index 4f9083155..3c1dbf6c0 100644 --- a/lib/connections/quic_listen.go +++ b/lib/connections/quic_listen.go @@ -9,13 +9,13 @@ package connections import ( + "context" "crypto/tls" "net" "net/url" "strings" "sync" "sync/atomic" - "time" "github.com/lucas-clemente/quic-go" @@ -80,6 +80,10 @@ func (t *quicListener) OnExternalAddressChanged(address *stun.Host, via string) func (t *quicListener) serve(stop chan struct{}) error { network := strings.Replace(t.uri.Scheme, "quic", "udp", -1) + // Convert the stop channel into a context + ctx, cancel := context.WithCancel(context.Background()) + go func() { <-stop; cancel() }() + packetConn, err := net.ListenPacket(network, t.uri.Host) if err != nil { l.Infoln("Listen (BEP/quic):", err) @@ -101,57 +105,32 @@ func (t *quicListener) serve(stop chan struct{}) error { l.Infoln("Listen (BEP/quic):", err) return err } + defer listener.Close() l.Infof("QUIC listener (%v) starting", packetConn.LocalAddr()) defer l.Infof("QUIC listener (%v) shutting down", packetConn.LocalAddr()) - // Accept is forever, so handle stops externally. - go func() { - select { - case <-stop: - _ = listener.Close() - } - }() - for { - // Blocks forever, see https://github.com/lucas-clemente/quic-go/issues/1915 - session, err := listener.Accept() - select { - case <-stop: - if err == nil { - _ = session.Close() - } + case <-ctx.Done(): return nil default: } - if err != nil { - if err, ok := err.(net.Error); !ok || !err.Timeout() { - l.Warnln("Listen (BEP/quic): Accepting connection:", err) - } + + session, err := listener.Accept(ctx) + if err == context.Canceled { + return nil + } else if err != nil { + l.Warnln("Listen (BEP/quic): Accepting connection:", err) continue } - l.Debugln("connect from", session.RemoteAddr()) - // Accept blocks forever, give it 10s to do it's thing. - ok := make(chan struct{}) - go func() { - select { - case <-ok: - return - case <-stop: - _ = session.Close() - case <-time.After(10 * time.Second): - l.Debugln("timed out waiting for AcceptStream on", session.RemoteAddr()) - _ = session.Close() - } - }() - - stream, err := session.AcceptStream() - close(ok) + streamCtx, cancel := context.WithTimeout(ctx, quicOperationTimeout) + stream, err := session.AcceptStream(streamCtx) + cancel() if err != nil { - l.Debugln("failed to accept stream from", session.RemoteAddr(), err.Error()) + l.Debugf("failed to accept stream from %s: %v", session.RemoteAddr(), err) _ = session.Close() continue }