From 97dd44e2019c8257c8fe782f48c0a5770d01a195 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 18 Dec 2024 09:54:21 -0800 Subject: [PATCH] http2, internal/gate: move Gate type to an internal package For reuse in internal/http3. Change-Id: I186d7219194a07c100aa8bd049e007232de2d3d9 Reviewed-on: https://go-review.googlesource.com/c/net/+/641497 Auto-Submit: Damien Neil LUCI-TryBot-Result: Go LUCI Reviewed-by: Jonathan Amsterdam --- http2/clientconn_test.go | 15 ++-- http2/gate_test.go => internal/gate/gate.go | 47 +++++------- internal/gate/gate_test.go | 85 +++++++++++++++++++++ 3 files changed, 112 insertions(+), 35 deletions(-) rename http2/gate_test.go => internal/gate/gate.go (52%) create mode 100644 internal/gate/gate_test.go diff --git a/http2/clientconn_test.go b/http2/clientconn_test.go index 42d9fd2dcc..f9e9a2fdaa 100644 --- a/http2/clientconn_test.go +++ b/http2/clientconn_test.go @@ -20,6 +20,7 @@ import ( "time" "golang.org/x/net/http2/hpack" + "golang.org/x/net/internal/gate" ) // TestTestClientConn demonstrates usage of testClientConn. @@ -206,7 +207,7 @@ func (tc *testClientConn) closeWrite() { // testRequestBody is a Request.Body for use in tests. type testRequestBody struct { tc *testClientConn - gate gate + gate gate.Gate // At most one of buf or bytes can be set at any given time: buf bytes.Buffer // specific bytes to read from the body @@ -218,18 +219,18 @@ type testRequestBody struct { func (tc *testClientConn) newRequestBody() *testRequestBody { b := &testRequestBody{ tc: tc, - gate: newGate(), + gate: gate.New(false), } return b } func (b *testRequestBody) unlock() { - b.gate.unlock(b.buf.Len() > 0 || b.bytes > 0 || b.err != nil) + b.gate.Unlock(b.buf.Len() > 0 || b.bytes > 0 || b.err != nil) } // Read is called by the ClientConn to read from a request body. func (b *testRequestBody) Read(p []byte) (n int, _ error) { - if err := b.gate.waitAndLock(context.Background()); err != nil { + if err := b.gate.WaitAndLock(context.Background()); err != nil { return 0, err } defer b.unlock() @@ -258,7 +259,7 @@ func (b *testRequestBody) Close() error { // writeBytes adds n arbitrary bytes to the body. func (b *testRequestBody) writeBytes(n int) { defer b.tc.sync() - b.gate.lock() + b.gate.Lock() defer b.unlock() b.bytes += n b.checkWrite() @@ -268,7 +269,7 @@ func (b *testRequestBody) writeBytes(n int) { // Write adds bytes to the body. func (b *testRequestBody) Write(p []byte) (int, error) { defer b.tc.sync() - b.gate.lock() + b.gate.Lock() defer b.unlock() n, err := b.buf.Write(p) b.checkWrite() @@ -287,7 +288,7 @@ func (b *testRequestBody) checkWrite() { // closeWithError sets an error which will be returned by Read. func (b *testRequestBody) closeWithError(err error) { defer b.tc.sync() - b.gate.lock() + b.gate.Lock() defer b.unlock() b.err = err } diff --git a/http2/gate_test.go b/internal/gate/gate.go similarity index 52% rename from http2/gate_test.go rename to internal/gate/gate.go index 9b18bccfbb..5c026c002d 100644 --- a/http2/gate_test.go +++ b/internal/gate/gate.go @@ -1,40 +1,37 @@ // Copyright 2024 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. -package http2 + +// Package gate contains an alternative condition variable. +package gate import "context" -// An gate is a monitor (mutex + condition variable) with one bit of state. +// A Gate is a monitor (mutex + condition variable) with one bit of state. // // The condition may be either set or unset. // Lock operations may be unconditional, or wait for the condition to be set. // Unlock operations record the new state of the condition. -type gate struct { +type Gate struct { // When unlocked, exactly one of set or unset contains a value. // When locked, neither chan contains a value. set chan struct{} unset chan struct{} } -// newGate returns a new, unlocked gate with the condition unset. -func newGate() gate { - g := newLockedGate() - g.unlock(false) - return g -} - -// newLockedGate returns a new, locked gate. -func newLockedGate() gate { - return gate{ +// New returns a new, unlocked gate. +func New(set bool) Gate { + g := Gate{ set: make(chan struct{}, 1), unset: make(chan struct{}, 1), } + g.Unlock(set) + return g } -// lock acquires the gate unconditionally. +// Lock acquires the gate unconditionally. // It reports whether the condition is set. -func (g *gate) lock() (set bool) { +func (g *Gate) Lock() (set bool) { select { case <-g.set: return true @@ -43,9 +40,9 @@ func (g *gate) lock() (set bool) { } } -// waitAndLock waits until the condition is set before acquiring the gate. -// If the context expires, waitAndLock returns an error and does not acquire the gate. -func (g *gate) waitAndLock(ctx context.Context) error { +// WaitAndLock waits until the condition is set before acquiring the gate. +// If the context expires, WaitAndLock returns an error and does not acquire the gate. +func (g *Gate) WaitAndLock(ctx context.Context) error { select { case <-g.set: return nil @@ -59,8 +56,8 @@ func (g *gate) waitAndLock(ctx context.Context) error { } } -// lockIfSet acquires the gate if and only if the condition is set. -func (g *gate) lockIfSet() (acquired bool) { +// LockIfSet acquires the gate if and only if the condition is set. +func (g *Gate) LockIfSet() (acquired bool) { select { case <-g.set: return true @@ -69,17 +66,11 @@ func (g *gate) lockIfSet() (acquired bool) { } } -// unlock sets the condition and releases the gate. -func (g *gate) unlock(set bool) { +// Unlock sets the condition and releases the gate. +func (g *Gate) Unlock(set bool) { if set { g.set <- struct{}{} } else { g.unset <- struct{}{} } } - -// unlockFunc sets the condition to the result of f and releases the gate. -// Useful in defers. -func (g *gate) unlockFunc(f func() bool) { - g.unlock(f()) -} diff --git a/internal/gate/gate_test.go b/internal/gate/gate_test.go new file mode 100644 index 0000000000..87a78b15af --- /dev/null +++ b/internal/gate/gate_test.go @@ -0,0 +1,85 @@ +// Copyright 2024 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. + +package gate_test + +import ( + "context" + "testing" + "time" + + "golang.org/x/net/internal/gate" +) + +func TestGateLockAndUnlock(t *testing.T) { + g := gate.New(false) + if set := g.Lock(); set { + t.Errorf("g.Lock of never-locked gate: true, want false") + } + unlockedc := make(chan struct{}) + donec := make(chan struct{}) + go func() { + defer close(donec) + if set := g.Lock(); !set { + t.Errorf("g.Lock of set gate: false, want true") + } + select { + case <-unlockedc: + default: + t.Errorf("g.Lock succeeded while gate was held") + } + g.Unlock(false) + }() + time.Sleep(1 * time.Millisecond) + close(unlockedc) + g.Unlock(true) + <-donec + if set := g.Lock(); set { + t.Errorf("g.Lock of unset gate: true, want false") + } +} + +func TestGateWaitAndLock(t *testing.T) { + g := gate.New(false) + // WaitAndLock is canceled. + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) + defer cancel() + if err := g.WaitAndLock(ctx); err != context.DeadlineExceeded { + t.Fatalf("g.WaitAndLock = %v, want context.DeadlineExceeded", err) + } + // WaitAndLock succeeds. + set := false + go func() { + time.Sleep(1 * time.Millisecond) + g.Lock() + set = true + g.Unlock(true) + }() + if err := g.WaitAndLock(context.Background()); err != nil { + t.Fatalf("g.WaitAndLock = %v, want nil", err) + } + if !set { + t.Fatalf("g.WaitAndLock returned before gate was set") + } + g.Unlock(true) + // WaitAndLock succeeds when the gate is set and the context is canceled. + if err := g.WaitAndLock(ctx); err != nil { + t.Fatalf("g.WaitAndLock = %v, want nil", err) + } +} + +func TestGateLockIfSet(t *testing.T) { + g := gate.New(false) + if locked := g.LockIfSet(); locked { + t.Fatalf("g.LockIfSet of unset gate = %v, want false", locked) + } + g.Lock() + if locked := g.LockIfSet(); locked { + t.Fatalf("g.LockIfSet of locked gate = %v, want false", locked) + } + g.Unlock(true) + if locked := g.LockIfSet(); !locked { + t.Fatalf("g.LockIfSet of set gate = %v, want true", locked) + } +}