fix(chromium): serialize browser starts to prevent pinning proxy latch after a start timeout

This commit is contained in:
Julien Neuhart
2026-07-15 19:04:38 +02:00
parent d67ef724f9
commit d0e3991d16
2 changed files with 60 additions and 0 deletions

View File

@@ -58,6 +58,14 @@ type chromiumBrowser struct {
userProfileDirPath string userProfileDirPath string
ctxMu sync.RWMutex ctxMu sync.RWMutex
isStarted atomic.Bool isStarted atomic.Bool
// startMu serializes Start calls. The supervisor's runWithDeadline
// abandons a Start goroutine when the request deadline expires while
// Chromium's startup handshake is still hanging; the abandoned goroutine
// keeps running, holding the resources it acquired (the pinning proxy).
// Serializing here prevents a second, overlapping Start from colliding
// with the in-flight one on the shared pinning proxy.
// See https://github.com/gotenberg/gotenberg/issues/1599.
startMu sync.Mutex
arguments browserArguments arguments browserArguments
fs *gotenberg.FileSystem fs *gotenberg.FileSystem
@@ -77,6 +85,19 @@ func newChromiumBrowser(arguments browserArguments) browser {
} }
func (b *chromiumBrowser) Start(logger *slog.Logger) error { func (b *chromiumBrowser) Start(logger *slog.Logger) error {
// Refuse to run while a previous Start is still in flight. That previous
// Start may be a goroutine the supervisor abandoned after the request
// deadline expired while the Chromium startup handshake was hanging; it
// still holds the pinning proxy it started. An abandoned goroutine keeps
// holding startMu until it unwinds (bounded by --chromium-start-timeout),
// so no overlapping Start can collide with it on the shared pinning proxy
// and latch Chromium into a permanent "pinning proxy already started"
// state. See https://github.com/gotenberg/gotenberg/issues/1599.
if !b.startMu.TryLock() {
return errors.New("browser start already in progress")
}
defer b.startMu.Unlock()
if b.isStarted.Load() { if b.isStarted.Load() {
return errors.New("browser is already started") return errors.New("browser is already started")
} }

View File

@@ -0,0 +1,39 @@
package chromium
import (
"context"
"log/slog"
"strings"
"testing"
)
// TestChromiumBrowser_Start_rejectsOverlappingStart guards against the latch
// reported in https://github.com/gotenberg/gotenberg/issues/1599. When the
// supervisor abandons a Start goroutine on request-deadline expiry, that
// goroutine keeps running and holds startMu (and the pinning proxy it started)
// until it unwinds. A second Start must be refused rather than proceeding to
// start the pinning proxy a second time.
func TestChromiumBrowser_Start_rejectsOverlappingStart(t *testing.T) {
b := &chromiumBrowser{initialCtx: context.Background()}
// Simulate a Start still in flight.
b.startMu.Lock()
defer b.startMu.Unlock()
err := b.Start(slog.New(slog.DiscardHandler))
if err == nil {
t.Fatal("expected an error when a start is already in progress, got nil")
}
if !strings.Contains(err.Error(), "already in progress") {
t.Fatalf("expected an 'already in progress' error, got %q", err)
}
// The guard must return before touching any startup resource, so no user
// profile directory is created and the browser stays not started.
if b.userProfileDirPath != "" {
t.Fatalf("expected no user profile directory to be created, got %q", b.userProfileDirPath)
}
if b.isStarted.Load() {
t.Fatal("expected the browser to stay not started")
}
}