fix: harden retryable errors, pool shutdown race, session lock probe, and minor cleanups

- Prevent Acquire from leaking WebViews when pool closes during creation
- Use CrashPath constant instead of hardcoded "/crash" string
- Fix log field "url" -> "uri" for consistency with parameter name
- Broaden isRetryableRequestError to detect transient network errors
  (ECONNRESET, ECONNREFUSED, ENETUNREACH, EHOSTUNREACH, EADDRNOTAVAIL)
- Replace os.Stat lock check in markAbruptExits with non-blocking lock
  probe to avoid misclassifying stale locks as active sessions

commit: 36ecc6b7c2fc3f5c01c148b5124ebd06571107af
author: brice · 8mo ago · 2026-02-07 10:55 UTC
parent: 96f2b51
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

5 files · +75 -11

M internal/bootstrap/session.go +26 -8


@@ -354,18 +354,36 @@ func markAbruptExits(lockDir string, detectedAt time.Time, logger *zerolog.Logge
354	354		}
355	355
356	356		lockPath := sessionLockPath(lockDir, entity.SessionID(sessionID))
357		-	if _, err := os.Stat(lockPath); err == nil {
358		-	// Session still actively running; do not mark as abrupt.
359		-	continue
360		-	} else if !os.IsNotExist(err) {
361		-	if logger != nil {
	357	+	f, openErr := os.OpenFile(lockPath, os.O_RDWR, lockFilePerm)
	358	+	if openErr != nil {
	359	+	if !os.IsNotExist(openErr) && logger != nil {
362	360		logger.Warn().
363		-	Err(err).
	361	+	Err(openErr).
364	362		Str("session_id", sessionID).
365	363		Str("path", lockPath).
366		-	Msg("markAbruptExits: stat failed")
	364	+	Msg("markAbruptExits: open lock file failed")
367	365		}
368		-	continue
	366	+	// No lock file or open error: fall through to mark as abrupt.
	367	+	} else {
	368	+	locked, lockErr := tryLockExclusiveNonBlocking(f)
	369	+	if lockErr != nil {
	370	+	_ = f.Close()
	371	+	if logger != nil {
	372	+	logger.Warn().
	373	+	Err(lockErr).
	374	+	Str("session_id", sessionID).
	375	+	Str("path", lockPath).
	376	+	Msg("markAbruptExits: lock probe failed")
	377	+	}
	378	+	continue
	379	+	}
	380	+	if !locked {
	381	+	// Lock is held by another process: session still active.
	382	+	_ = f.Close()
	383	+	continue
	384	+	}
	385	+	// Lock acquired: no process holds it, session is stale.
	386	+	_ = unlockAndClose(f)
369	387		}
370	388
371	389		shutdownPath := shutdownMarkerPath(lockDir, sessionID)


@@ -354,18 +354,36 @@ func markAbruptExits(lockDir string, detectedAt time.Time, logger *zerolog.Logge
354		}	354		}
355			355
356		lockPath := sessionLockPath(lockDir, entity.SessionID(sessionID))	356		lockPath := sessionLockPath(lockDir, entity.SessionID(sessionID))
357	-	if _, err := os.Stat(lockPath); err == nil {	357	+	f, openErr := os.OpenFile(lockPath, os.O_RDWR, lockFilePerm)
358	-	// Session still actively running; do not mark as abrupt.	358	+	if openErr != nil {
359	-	continue	359	+	if !os.IsNotExist(openErr) && logger != nil {
360	-	} else if !os.IsNotExist(err) {
361	-	if logger != nil {
362		logger.Warn().	360		logger.Warn().
363	-	Err(err).	361	+	Err(openErr).
364		Str("session_id", sessionID).	362		Str("session_id", sessionID).
365		Str("path", lockPath).	363		Str("path", lockPath).
366	-	Msg("markAbruptExits: stat failed")	364	+	Msg("markAbruptExits: open lock file failed")
367		}	365		}
368	-	continue	366	+	// No lock file or open error: fall through to mark as abrupt.
			367	+	} else {
			368	+	locked, lockErr := tryLockExclusiveNonBlocking(f)
			369	+	if lockErr != nil {
			370	+	_ = f.Close()
			371	+	if logger != nil {
			372	+	logger.Warn().
			373	+	Err(lockErr).
			374	+	Str("session_id", sessionID).
			375	+	Str("path", lockPath).
			376	+	Msg("markAbruptExits: lock probe failed")
			377	+	}
			378	+	continue
			379	+	}
			380	+	if !locked {
			381	+	// Lock is held by another process: session still active.
			382	+	_ = f.Close()
			383	+	continue
			384	+	}
			385	+	// Lock acquired: no process holds it, session is stale.
			386	+	_ = unlockAndClose(f)
369		}	387		}
370			388
371		shutdownPath := shutdownMarkerPath(lockDir, sessionID)	389		shutdownPath := shutdownMarkerPath(lockDir, sessionID)

M internal/infrastructure/updater/github.go +39 -1


@@ -22,6 +22,7 @@ import (
22	22		"runtime"
23	23		"strconv"
24	24		"strings"
	25	+	"syscall"
25	26		"time"
26	27
27	28		"github.com/bnema/dumber/internal/application/port"

@@ -272,8 +273,45 @@ func isRetryableRequestError(err error) bool {
272	273		if errors.Is(err, context.DeadlineExceeded) {
273	274		return true
274	275		}
	276	+
275	277		var netErr net.Error
276		-	return errors.As(err, &netErr) && netErr.Timeout()
	278	+	if errors.As(err, &netErr) {
	279	+	if netErr.Timeout() {
	280	+	return true
	281	+	}
	282	+	//nolint:staticcheck // net.Error.Temporary is deprecated but still useful for transient detection
	283	+	if netErr.Temporary() {
	284	+	return true
	285	+	}
	286	+	}
	287	+
	288	+	var opErr *net.OpError
	289	+	if errors.As(err, &opErr) {
	290	+	if isTransientSyscallError(opErr.Err) {
	291	+	return true
	292	+	}
	293	+	}
	294	+
	295	+	var urlErr *url.Error
	296	+	if errors.As(err, &urlErr) && urlErr.Temporary() {
	297	+	return true
	298	+	}
	299	+
	300	+	return false
	301	+	}
	302	+
	303	+	func isTransientSyscallError(err error) bool {
	304	+	var errno syscall.Errno
	305	+	if !errors.As(err, &errno) {
	306	+	return false
	307	+	}
	308	+	switch errno {
	309	+	case syscall.ECONNRESET, syscall.ECONNREFUSED,
	310	+	syscall.EADDRNOTAVAIL, syscall.ENETUNREACH,
	311	+	syscall.EHOSTUNREACH:
	312	+	return true
	313	+	}
	314	+	return false
277	315		}
278	316
279	317		func waitForBackoff(ctx context.Context, d time.Duration) error {


@@ -22,6 +22,7 @@ import (
22		"runtime"	22		"runtime"
23		"strconv"	23		"strconv"
24		"strings"	24		"strings"
			25	+	"syscall"
25		"time"	26		"time"
26			27
27		"github.com/bnema/dumber/internal/application/port"	28		"github.com/bnema/dumber/internal/application/port"

@@ -272,8 +273,45 @@ func isRetryableRequestError(err error) bool {
272		if errors.Is(err, context.DeadlineExceeded) {	273		if errors.Is(err, context.DeadlineExceeded) {
273		return true	274		return true
274		}	275		}
			276	+
275		var netErr net.Error	277		var netErr net.Error
276	-	return errors.As(err, &netErr) && netErr.Timeout()	278	+	if errors.As(err, &netErr) {
			279	+	if netErr.Timeout() {
			280	+	return true
			281	+	}
			282	+	//nolint:staticcheck // net.Error.Temporary is deprecated but still useful for transient detection
			283	+	if netErr.Temporary() {
			284	+	return true
			285	+	}
			286	+	}
			287	+
			288	+	var opErr *net.OpError
			289	+	if errors.As(err, &opErr) {
			290	+	if isTransientSyscallError(opErr.Err) {
			291	+	return true
			292	+	}
			293	+	}
			294	+
			295	+	var urlErr *url.Error
			296	+	if errors.As(err, &urlErr) && urlErr.Temporary() {
			297	+	return true
			298	+	}
			299	+
			300	+	return false
			301	+	}
			302	+
			303	+	func isTransientSyscallError(err error) bool {
			304	+	var errno syscall.Errno
			305	+	if !errors.As(err, &errno) {
			306	+	return false
			307	+	}
			308	+	switch errno {
			309	+	case syscall.ECONNRESET, syscall.ECONNREFUSED,
			310	+	syscall.EADDRNOTAVAIL, syscall.ENETUNREACH,
			311	+	syscall.EHOSTUNREACH:
			312	+	return true
			313	+	}
			314	+	return false
277		}	315		}
278			316
279		func waitForBackoff(ctx context.Context, d time.Duration) error {	317		func waitForBackoff(ctx context.Context, d time.Duration) error {

M internal/infrastructure/webkit/pool.go +8 -0


@@ -196,6 +196,14 @@ func (p WebViewPool) Acquire(ctx context.Context) (WebView, error) {
196	196		if err != nil {
197	197		return nil, err
198	198		}
	199	+
	200	+	// Re-check after creation: if pool closed during createWebView, destroy
	201	+	// the new WebView and return an error to avoid leaking resources.
	202	+	if p.closed.Load() {
	203	+	wv.DestroyWithPolicy(p.terminatePolicy)
	204	+	return nil, ErrPoolClosed
	205	+	}
	206	+
199	207		log.Debug().
200	208		Uint64("id", uint64(wv.ID())).
201	209		Dur("latency", time.Since(start)).


@@ -196,6 +196,14 @@ func (p WebViewPool) Acquire(ctx context.Context) (WebView, error) {
196	if err != nil {	196		if err != nil {
197	return nil, err	197		return nil, err
198	}	198		}
		199	+
		200	+	// Re-check after creation: if pool closed during createWebView, destroy
		201	+	// the new WebView and return an error to avoid leaking resources.
		202	+	if p.closed.Load() {
		203	+	wv.DestroyWithPolicy(p.terminatePolicy)
		204	+	return nil, ErrPoolClosed
		205	+	}
		206	+
199	log.Debug().	207		log.Debug().
200	Uint64("id", uint64(wv.ID())).	208		Uint64("id", uint64(wv.ID())).
201	Dur("latency", time.Since(start)).	209		Dur("latency", time.Since(start)).

M internal/infrastructure/webkit/scheme_handler.go +1 -1


@@ -165,7 +165,7 @@ func (h *DumbSchemeHandler) registerDefaults() {
165	165		}))
166	166
167	167		// Crash page (web process termination fallback)
168		-	h.RegisterPage("/crash", PageHandlerFunc(func(req SchemeRequest) SchemeResponse {
	168	+	h.RegisterPage("/"+CrashPath, PageHandlerFunc(func(req SchemeRequest) SchemeResponse {
169	169		if req.Method != "" && req.Method != httpGET {
170	170		return nil
171	171		}


@@ -165,7 +165,7 @@ func (h *DumbSchemeHandler) registerDefaults() {
165		}))	165		}))
166			166
167		// Crash page (web process termination fallback)	167		// Crash page (web process termination fallback)
168	-	h.RegisterPage("/crash", PageHandlerFunc(func(req SchemeRequest) SchemeResponse {	168	+	h.RegisterPage("/"+CrashPath, PageHandlerFunc(func(req SchemeRequest) SchemeResponse {
169		if req.Method != "" && req.Method != httpGET {	169		if req.Method != "" && req.Method != httpGET {
170		return nil	170		return nil
171		}	171		}

M internal/ui/coordinator/content.go +1 -1


@@ -1205,7 +1205,7 @@ func (c *ContentCoordinator) revealIfPending(ctx context.Context, paneID entity.
1205	1205		logging.FromContext(ctx).
1206	1206		Debug().
1207	1207		Str("pane_id", string(paneID)).
1208		-	Str("url", uri).
	1208	+	Str("uri", uri).
1209	1209		Str("reason", reason).
1210	1210		Msg("webview revealed")
1211	1211		}


@@ -1205,7 +1205,7 @@ func (c *ContentCoordinator) revealIfPending(ctx context.Context, paneID entity.
1205		logging.FromContext(ctx).	1205		logging.FromContext(ctx).
1206		Debug().	1206		Debug().
1207		Str("pane_id", string(paneID)).	1207		Str("pane_id", string(paneID)).
1208	-	Str("url", uri).	1208	+	Str("uri", uri).
1209		Str("reason", reason).	1209		Str("reason", reason).
1210		Msg("webview revealed")	1210		Msg("webview revealed")
1211		}	1211		}