fix: bound opening a Chrome tab

Render bounds opening the tab and rendering the page with one deadline
(TabOpenTimeout 5 s plus RenderTimeout 20 s), so a hung Chrome no longer
holds the crawl's render lock until the job timeout.

classes/crawl/render_chrome.go

@@ -7,8 +7,10 @@ import (
 	"net/netip"
 	"os"
 	"sort"
+	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"git.golem15.com/golem15/summercms/modules/fetchguard"
 	"github.com/chromedp/cdproto/browser"
@@ -31,8 +33,11 @@ import (
 //     BlockedByClient, so third-party scripts are never loaded;
 //   - a fresh temporary profile per crawl, removed by Close; no cookies,
 //     credentials or extensions; downloads denied;
-//   - one tab at a time (the crawler serialises Render), 20 s per page, the
-//     DOM capped at 5 MiB; Close stops Chrome.
+//   - one tab at a time (the crawler serialises Render), at most
+//     TabOpenTimeout (5 s) to open the tab and RenderTimeout (20 s) for the
+//     page, both under one deadline, so a hung Chrome never holds the
+//     crawl's render lock longer; the DOM capped at 5 MiB; Close stops
+//     Chrome.
 //
 // chromedp's default allocator flags are used as they are: nothing is
 // added that weakens Chrome's sandbox.
@@ -45,11 +50,30 @@ type chromeRenderer struct {
 	browserCtx    context.Context
 	browserCancel context.CancelFunc
 
+	// newTab and startTab open a tab: chromedp.NewContext and chromedp.Do
+	// unless a test replaces them.
+	newTab   func(parent context.Context) (context.Context, context.CancelFunc)
+	startTab func(tab context.Context) error
+
 	mu      sync.Mutex
 	blocked []string
 	closed  bool
 }
 
+// TabOpenTimeout bounds opening a Chrome tab (P33-60, L3 WR-06); Render
+// bounds opening the tab and rendering the page with one deadline of
+// TabOpenTimeout plus RenderTimeout.
+var TabOpenTimeout = 5 * time.Second
+
+// seconds writes d as whole seconds ("5 s") when it is one, else as
+// time.Duration does.
+func seconds(d time.Duration) string {
+	if d%time.Second == 0 {
+		return strconv.Itoa(int(d/time.Second)) + " s"
+	}
+	return d.String()
+}
+
 // newChromeRenderer starts Chrome at path for the hosts and their checked
 // addresses.
 func newChromeRenderer(path string, hosts map[string][]netip.Addr, trusted bool, ua string) (*chromeRenderer, error) {
@@ -96,14 +120,42 @@ func (r *chromeRenderer) Render(ctx context.Context, pageURL string) (string, er
 	if !allowRequest(pageURL, r.allowed, r.trusted) {
 		return "", fmt.Errorf("the address %s is outside the site", pageURL)
 	}
-	tab, closeTab := chromedp.NewContext(r.browserCtx)
+	// One deadline bounds opening the tab and rendering the page; the tab
+	// ends with it or with the crawl's context (a cancelled crawl).
+	rctx, rcancel := context.WithTimeout(ctx, TabOpenTimeout+RenderTimeout)
+	defer rcancel()
+	newTab, startTab := r.newTab, r.startTab
+	if newTab == nil {
+		newTab = func(parent context.Context) (context.Context, context.CancelFunc) { return chromedp.NewContext(parent) }
+	}
+	if startTab == nil {
+		startTab = func(tab context.Context) error { return chromedp.Do(tab) }
+	}
+	tab, closeTab := newTab(r.browserCtx)
 	defer closeTab()
-	// The tab ends with the crawl's context (a cancelled crawl) or after
-	// the page's timeout.
-	stop := context.AfterFunc(ctx, closeTab)
+	stop := context.AfterFunc(rctx, closeTab)
 	defer stop()
-	if err := chromedp.Do(tab); err != nil {
-		return "", fmt.Errorf("Chrome cannot open a tab: %w", err)
+	opened := make(chan error, 1)
+	go func() { opened <- startTab(tab) }()
+	openTimer := time.NewTimer(TabOpenTimeout)
+	defer openTimer.Stop()
+	select {
+	case err := <-opened:
+		if err != nil {
+			if ctx.Err() != nil {
+				return "", ctx.Err()
+			}
+			return "", fmt.Errorf("Chrome cannot open a tab: %w", err)
+		}
+	case <-openTimer.C:
+		closeTab()
+		return "", fmt.Errorf("Chrome did not open a tab within %s", seconds(TabOpenTimeout))
+	case <-rctx.Done():
+		closeTab()
+		if ctx.Err() != nil {
+			return "", ctx.Err()
+		}
+		return "", fmt.Errorf("Chrome did not open a tab within %s", seconds(TabOpenTimeout))
 	}
 	page, cancel := context.WithTimeout(tab, RenderTimeout)
 	defer cancel()

classes/crawl/render_test.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"git.golem15.com/golem15/summercms/modules/compass"
 )
@@ -124,3 +125,42 @@ func TestRenderMax(t *testing.T) {
 		t.Fatalf("SiteHosts = %v", got)
 	}
 }
+
+// TestRenderTabTimeout pins L3 WR-06: a tab that does not open answers an
+// error within TabOpenTimeout (lowered for the test) plus a margin, so the
+// crawl's render lock is released; the next Render is not held up by the
+// first one's hung tab.
+func TestRenderTabTimeout(t *testing.T) {
+	saved := TabOpenTimeout
+	TabOpenTimeout = 50 * time.Millisecond
+	t.Cleanup(func() { TabOpenTimeout = saved })
+	hung := make(chan struct{})
+	t.Cleanup(func() { close(hung) })
+	r := &chromeRenderer{
+		allowed:    []string{"example.test"},
+		browserCtx: context.Background(),
+		newTab: func(parent context.Context) (context.Context, context.CancelFunc) {
+			return context.WithCancel(parent)
+		},
+		// A hung Chrome: opening the tab never answers, even when the tab
+		// is closed.
+		startTab: func(context.Context) error {
+			<-hung
+			return nil
+		},
+	}
+	for i := range 2 {
+		start := time.Now()
+		_, err := r.Render(t.Context(), "https://example.test/a")
+		if err == nil || !strings.Contains(err.Error(), "did not open a tab within 50ms") {
+			t.Fatalf("Render %d: %v, want the tab timeout", i, err)
+		}
+		if d := time.Since(start); d > time.Second {
+			t.Fatalf("Render %d returned after %v", i, d)
+		}
+	}
+	TabOpenTimeout = 5 * time.Second
+	if got := seconds(TabOpenTimeout); got != "5 s" {
+		t.Fatalf("seconds = %q", got)
+	}
+}