diff --git a/broadcast.go b/broadcast.go index 15522f9f..ae94fd4d 100644 --- a/broadcast.go +++ b/broadcast.go @@ -188,6 +188,9 @@ func (jw *Jaws) distributeDirt() int { } // Reload requests all active [Request] values to reload their current page. +// +// Each WebSocket closes after sending Reload. See [Jaws.Broadcast] for the +// processing-loop requirement. func (jw *Jaws) Reload() { jw.Broadcast(wire.Message{ What: what.Reload, diff --git a/doc/browser.md b/doc/browser.md index 5151e990..dd745c2b 100644 --- a/doc/browser.md +++ b/doc/browser.md @@ -117,6 +117,9 @@ root, replace a subtree, or delete an object property. ## Connection loss +A server-requested reload stops connection recovery before reloading the page. +It does not show the connection-loss indicator. + After a WebSocket failure, the client waits five seconds before probing `/jaws/.ping`. A successful probe reloads a page whose navigation age is at least 60 seconds. Otherwise, the client shows a connection-loss indicator and retries diff --git a/doc/runtime.md b/doc/runtime.md index 0e706606..2a5e0686 100644 --- a/doc/runtime.md +++ b/doc/runtime.md @@ -70,6 +70,10 @@ Browser event handlers run serially within each Request, concurrently with updat processing and handlers in other Requests. Synchronize shared application state and return promptly. +Queued browser commands wake the Request processing loop. Maintenance also wakes +Requests with queued output. State-driven rendering still uses `Dirty` and the +normal update interval. + An event accepted before its target is removed can still reach that deleted Element. Its render, update, and queue helpers become no-ops. Browser events while disconnected are not replayed; see [browser integration](browser.md). diff --git a/doc/sessions.md b/doc/sessions.md index 5f125fcc..68c81d75 100644 --- a/doc/sessions.md +++ b/doc/sessions.md @@ -72,6 +72,12 @@ cookie. See [session.go](../session.go) for these contracts. `Jaws.Close` invalidates all Sessions and clears their data. +`Request.Reload` queues a reload command for one page. The WebSocket writer sends +it and closes the connection, cancelling the Request's context. Events and +callbacks run normally until disconnection. Pending pages send the reload after +successful connection setup, including their connection callback. Session data +and cookies are preserved. + `MaxSessions` limits registered Sessions globally. `MaxSessionsPerIP` limits a client address bucket. Both default to zero, which disables the respective limit. A lower per-IP cap reserves capacity for other buckets. Replacement needs diff --git a/doc/testing.md b/doc/testing.md index ea4f56d6..79f414e9 100644 --- a/doc/testing.md +++ b/doc/testing.md @@ -167,7 +167,6 @@ defer func() { <-drained }() -tr.BcastCh <- wire.Message{What: what.Update} // Flush queued initial adjustments. synctest.Wait() time.Sleep(jaws.DefaultUpdateInterval + time.Millisecond) synctest.Wait() @@ -179,11 +178,11 @@ synctest.Wait() // Let the dirty pass and collector finish. // Inspect records here, while the other goroutines are blocked. ``` -The unaddressed Update wakes the Request to flush initial render output; -`synctest.Wait` supplies synchronization. Fake time then crosses the normal -dirty-update interval without a real-time delay. This settles a synchronous -handler followed by one dirty pass. Advance application timers or release -application-owned gates explicitly when an action schedules additional work. +Queued output wakes the Request automatically; `synctest.Wait` supplies +synchronization. Fake time then crosses the normal dirty-update interval without +a real-time delay. This settles a synchronous handler followed by one dirty pass. +Advance application timers or release application-owned gates explicitly when an +action schedules additional work. After the final wait, compare the complete collected records with the expected commands and targets, including the absence of unrelated targets. Count diff --git a/doc/transport.md b/doc/transport.md index c2d1a32e..375b1a5a 100644 --- a/doc/transport.md +++ b/doc/transport.md @@ -105,6 +105,9 @@ These are logical payloads before wire quoting: | `SClass`, `RClass` | One class name | | `Value` | Live control value, not an HTML attribute value | +`Reload` is the final outbound record. The writer sends its batch and closes the +WebSocket, discarding any later records. + Browser `Input` and `JsVar` events invoke `JawsInput` on their Element. `Input` carries the control value. `JsVar` carries a `path=json` proposal. Click and context-menu events carry coordinates, modifier state, the nearest name, and diff --git a/element.go b/element.go index ce321750..ce23202c 100644 --- a/element.go +++ b/element.go @@ -25,6 +25,10 @@ import ( // or updates, though its fields are left intact and its methods still operate on the // now finished Request. Request identities are never reused, so a retained Element // can never come to represent an unrelated connection. +// +// Browser commands queued by Element methods wake a running Request's processing loop. +// Event handlers run concurrently with updates; use [Request.Dirty] for changes +// that need to be applied through [Updater.JawsUpdate]. type Element struct { *Request // (read-only) the Request the Element belongs to // internals @@ -220,16 +224,6 @@ func (elem *Element) JsVar(data string) { // queue enqueues a wire message of the given type and data for this element on // its Request, tagged with the element's Jid. It is a no-op once the element has // been deleted. -// -// It is intended to be called while the element is rendering or updating; the -// message is appended to the Request's muQueue-guarded outbound queue and flushed -// the next time the processing loop runs a send pass. During rendering and updating -// that pass is imminent. Called from an event handler, however, the message is -// flushed only when the loop is next woken — by a broadcast, an incoming event, or a -// dirty-driven update — which on an otherwise-idle request is not guaranteed to -// happen promptly. The reliable event-driven path is therefore to mark the element -// dirty (see [Request.Dirty]), which schedules a [Updater.JawsUpdate] and the wakeup -// that delivers it. func (elem *Element) queue(wht what.What, data string) (queued bool) { if !elem.deleted.Load() { elem.Request.queue(wire.WsMsg{ @@ -258,12 +252,6 @@ func isReservedAttr(attr string) bool { // The framework-owned "id" attribute is rejected (ASCII case-insensitively): // attempting to set it is reported as [ErrReservedAttribute] via reportMisuse and // nothing is sent, since it carries the [Element]'s JaWS identity. -// -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To change the [Element] in response to a browser event, mark it dirty -// with [Request.Dirty] instead: a change queued directly from an event handler is -// flushed only when the processing loop is next woken, which on an otherwise-idle -// request is not guaranteed to be prompt (see [Element.queue]). func (elem *Element) SetAttr(attr, value string) { if isReservedAttr(attr) { elem.Jaws.reportMisuse(fmt.Errorf("jaws: Element.SetAttr: %q: %w", attr, ErrReservedAttribute)) @@ -278,12 +266,6 @@ func (elem *Element) SetAttr(attr, value string) { // The framework-owned "id" attribute is rejected (ASCII case-insensitively): // attempting to remove it is reported as [ErrReservedAttribute] via reportMisuse and // nothing is sent, since it carries the [Element]'s JaWS identity. -// -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To change the [Element] in response to a browser event, mark it dirty -// with [Request.Dirty] instead: a change queued directly from an event handler is -// flushed only when the processing loop is next woken, which on an otherwise-idle -// request is not guaranteed to be prompt (see [Element.queue]). func (elem *Element) RemoveAttr(attr string) { if isReservedAttr(attr) { elem.Jaws.reportMisuse(fmt.Errorf("jaws: Element.RemoveAttr: %q: %w", attr, ErrReservedAttribute)) @@ -294,24 +276,12 @@ func (elem *Element) RemoveAttr(attr string) { // SetClass queues sending a class // to the browser for the [Element]. -// -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To change the [Element] in response to a browser event, mark it dirty -// with [Request.Dirty] instead: a change queued directly from an event handler is -// flushed only when the processing loop is next woken, which on an otherwise-idle -// request is not guaranteed to be prompt (see [Element.queue]). func (elem *Element) SetClass(cls string) { elem.queue(what.SClass, cls) } // RemoveClass queues sending a request to remove a class // to the browser for the [Element]. -// -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To change the [Element] in response to a browser event, mark it dirty -// with [Request.Dirty] instead: a change queued directly from an event handler is -// flushed only when the processing loop is next woken, which on an otherwise-idle -// request is not guaranteed to be prompt (see [Element.queue]). func (elem *Element) RemoveClass(cls string) { elem.queue(what.RClass, cls) } @@ -321,12 +291,6 @@ func (elem *Element) RemoveClass(cls string) { // When innerHTML exactly matches the browser's current serialized inner HTML, // JaWS leaves the existing descendants and their live state unchanged. Use // [Element.Replace] when matching markup must still create new nodes. -// -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To change the [Element] in response to a browser event, mark it dirty -// with [Request.Dirty] instead: a change queued directly from an event handler is -// flushed only when the processing loop is next woken, which on an otherwise-idle -// request is not guaranteed to be prompt. func (elem *Element) SetInner(innerHTML template.HTML) { elem.queue(what.Inner, string(innerHTML)) } @@ -334,12 +298,10 @@ func (elem *Element) SetInner(innerHTML template.HTML) { // SetValue queues sending a new current input value in textual form // to the browser for the [Element]. // -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To reconcile only this Element after a browser event, call +// To reconcile only this Element after a browser event, call // elem.Dirty(elem); this schedules JawsUpdate on the Request loop and serializes the // correction with other updates. Dirty a source tag instead when shared application -// state changed. Calling SetValue directly from an event handler may not flush -// promptly and bypasses that update ordering. +// state changed. Calling SetValue directly bypasses that update ordering. func (elem *Element) SetValue(value string) { elem.queue(what.Value, value) } @@ -351,10 +313,7 @@ func (elem *Element) SetValue(value string) { // jsfunc must be an application-controlled dot path; put user data in jsonstr, // not jsfunc. // -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent; a call queued directly from an event handler is only flushed when the -// processing loop is next woken. To call JavaScript for every element matching a -// tag, use [Jaws.JsCall]. +// To call JavaScript for every element matching a tag, use [Jaws.JsCall]. func (elem *Element) JsCall(jsfunc, jsonstr string) { elem.queue(what.Call, jsCallData(jsfunc, jsonstr)) } @@ -378,12 +337,6 @@ func (elem *Element) JsCall(jsfunc, jsonstr string) { // expected id attribute. If the guard does not find it, the call is a programming // error: debug builds panic and production builds report it via [Jaws.MustLog] // and skip the replacement. -// -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To change the [Element] in response to a browser event, mark it dirty -// with [Request.Dirty] instead: a change queued directly from an event handler is -// flushed only when the processing loop is next woken, which on an otherwise-idle -// request is not guaranteed to be prompt. func (elem *Element) Replace(htmlCode template.HTML) { if !elem.deleted.Load() { var b []byte @@ -400,12 +353,6 @@ func (elem *Element) Replace(htmlCode template.HTML) { } // Append appends a new HTML element as a child to the current one. -// -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To change the [Element] in response to a browser event, mark it dirty -// with [Request.Dirty] instead: a change queued directly from an event handler is -// flushed only when the processing loop is next woken, which on an otherwise-idle -// request is not guaranteed to be prompt (see [Element.queue]). func (elem *Element) Append(htmlCode template.HTML) { elem.queue(what.Append, string(htmlCode)) } @@ -417,7 +364,6 @@ func (elem *Element) Append(htmlCode template.HTML) { // command is queued. The browser also verifies that child is a direct DOM child // of elem before applying the insertion. // -// Call this while elem is rendering or updating, when a send pass is imminent. // To insert HTML at the same child index in every element matching a tag, use // [Jaws.Insert]. func (elem *Element) InsertBefore(child *Element, htmlCode template.HTML) { @@ -427,12 +373,6 @@ func (elem *Element) InsertBefore(child *Element, htmlCode template.HTML) { } // Order reorders the HTML elements. -// -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To change the [Element] in response to a browser event, mark it dirty -// with [Request.Dirty] instead: a change queued directly from an event handler is -// flushed only when the processing loop is next woken, which on an otherwise-idle -// request is not guaranteed to be prompt (see [Element.queue]). func (elem *Element) Order(jidList []jid.Jid) { if !elem.deleted.Load() && len(jidList) > 0 { var b []byte @@ -453,12 +393,6 @@ func (elem *Element) Order(jidList []jid.Jid) { // nor the registry is changed. The caller is responsible for ensuring child is // a direct DOM child of elem; the browser verifies that relationship before // applying the removal. -// -// Call this while the [Element] is rendering or updating, when a send pass is -// imminent. To change the [Element] in response to a browser event, mark it dirty -// with [Request.Dirty] instead: a change queued directly from an event handler is -// flushed only when the processing loop is next woken, which on an otherwise-idle -// request is not guaranteed to be prompt (see [Element.queue]). func (elem *Element) Remove(child *Element) { if elem.validChildElement("Remove", child) { if elem.queue(what.Remove, child.Jid().String()) { diff --git a/element_test.go b/element_test.go index ba82d295..a36958a9 100644 --- a/element_test.go +++ b/element_test.go @@ -259,9 +259,9 @@ func TestElement_Queued(t *testing.T) { th.NoErr(rq.UI(tss)) rq.Jaws.Dirty(tss) rq.Dirty(tss) - // The Serve loop only broadcasts what.Update when its updateTicker fires + // The Serve loop only distributes dirt when its updateTicker fires // (1ms in tests). Advance the fake clock past it, then let the process - // loop drain the update and invoke JawsUpdate. + // loop drain its dirty work and invoke JawsUpdate. time.Sleep(2 * time.Millisecond) synctest.Wait() n := atomic.LoadInt32(&tss.updateCalled) @@ -558,13 +558,6 @@ func TestElement_ReplaceMessageTargetsElementHTML(t *testing.T) { html := `
replaced
` elem.Replace(template.HTML(html)) - // Element.Replace queues directly on the Request, so poke the process loop - // once to ensure queued messages are flushed to OutCh in this harness. - select { - case rq.InCh <- wire.WsMsg{}: - case <-time.After(time.Second): - t.Fatal("timeout waking request process loop") - } msg := nextOutboundMsg(t, rq) if msg.What != what.Replace { diff --git a/examples/minesweeper/main_test.go b/examples/minesweeper/main_test.go index 57c62747..51c2f7b3 100644 --- a/examples/minesweeper/main_test.go +++ b/examples/minesweeper/main_test.go @@ -518,7 +518,6 @@ func TestLargeFloodUpdatesBothRequests(t *testing.T) { if err := tmpl.ExecuteTemplate(&body, "index.html", ui.With{RequestWriter: rw, Dot: g}); err != nil { t.Fatal(err) } - c.request.BcastCh <- wire.Message{What: what.Update} clients = append(clients, c) } settle := func() { diff --git a/lib/assets/jaws.js b/lib/assets/jaws.js index 654e43cc..55f93169 100644 --- a/lib/assets/jaws.js +++ b/lib/assets/jaws.js @@ -597,6 +597,7 @@ function jawsPerform(what, id, data) { data = JSON.parse(data); switch (what) { case 'Reload': + jawsUnloading(); window.location.reload(); return; case 'Redirect': diff --git a/lib/assets/js_test.go b/lib/assets/js_test.go index f16b7bd4..7fd374d8 100644 --- a/lib/assets/js_test.go +++ b/lib/assets/js_test.go @@ -1027,6 +1027,32 @@ process.stdout.write(JSON.stringify({reloads: reloads, errors: errors, state: ja } } +func TestJawsJS_ReloadStopsReconnect(t *testing.T) { + raw := runJawsJSSnippet(t, ` +let reloads = 0, closes = 0, timers = 0; +window.location.reload = function() { reloads++; }; +setTimeout = function() { timers++; }; +function FakeSocket() {} +FakeSocket.prototype.removeEventListener = function() {}; +FakeSocket.prototype.close = function() { closes++; }; +WebSocket = FakeSocket; +jaws = new FakeSocket(); +jawsMessage({data: 'Reload\t\t""\n'}); +jawsFailed(); +process.stdout.write(JSON.stringify({reloads, closes, timers, stopped: jaws === null})); +`) + var got struct { + Reloads, Closes, Timers int + Stopped bool + } + if err := json.Unmarshal([]byte(raw), &got); err != nil { + t.Fatal(err) + } + if got.Reloads != 1 || got.Closes != 1 || got.Timers != 0 || !got.Stopped { + t.Fatalf("reload handling = %+v", got) + } +} + func TestJawsJS_StorePathGuards(t *testing.T) { raw := runJawsJSSnippet(t, ` function FakeSocket() { this.readyState = 1; this.sent = []; } diff --git a/lib/ui/container_test.go b/lib/ui/container_test.go index 1b6cf06e..b42db5ae 100644 --- a/lib/ui/container_test.go +++ b/lib/ui/container_test.go @@ -109,8 +109,6 @@ func TestContainer_UpdateEmitsWireOps(t *testing.T) { tc.contents = []jaws.UI{span1, span2} elem.JawsUpdate() - // Wake the harness loop so the queued ops flush to OutCh. - tr.InCh <- wire.WsMsg{} synctest.Wait() var sawAppend, sawOrder bool @@ -409,7 +407,6 @@ func TestContainerUpdateRenderErrorDoesNotAppendFailedChild(t *testing.T) { t.Fatalf("failed append child %v leaked into the request registry", failingChild.jid) } - tr.InCh <- wire.WsMsg{} synctest.Wait() for { select { @@ -736,7 +733,6 @@ func TestSelectWidget_AppliesGetterAfterInitialRender(t *testing.T) { t.Fatalf("initial option markup unexpectedly selected an option: %s", got) } - tr.InCh <- wire.WsMsg{} select { case msg := <-tr.OutCh: if msg.What != what.Value || msg.Data != tc.want { @@ -791,7 +787,6 @@ func TestSelectWidget_AppendsOptionBeforeSettingNewValue(t *testing.T) { if err := elem.JawsRender(&sb, nil); err != nil { t.Fatal(err) } - tr.InCh <- wire.WsMsg{} select { case msg := <-tr.OutCh: if msg.What != what.Value || msg.Data != "1" { @@ -804,7 +799,6 @@ func TestSelectWidget_AppendsOptionBeforeSettingNewValue(t *testing.T) { sh.Set("2") sh.contents = []jaws.UI{opt1, opt2} selectUI.JawsUpdate(elem) - tr.InCh <- wire.WsMsg{} sawAppend := false for { diff --git a/lib/ui/input_widgets_test.go b/lib/ui/input_widgets_test.go index a7dd5515..27d21c58 100644 --- a/lib/ui/input_widgets_test.go +++ b/lib/ui/input_widgets_test.go @@ -126,7 +126,6 @@ func TestInputBool_JawsUpdateEmitsCheckedState(t *testing.T) { // false -> true emits "true". sb.Set(true) checkbox.JawsUpdate(elem) - tr.InCh <- wire.WsMsg{} // wake the loop so the queued op flushes to OutCh if v, ok := waitValue(); !ok || v != "true" { t.Fatalf("expected SetValue %q on false->true, got ok=%v v=%q", "true", ok, v) } @@ -134,7 +133,6 @@ func TestInputBool_JawsUpdateEmitsCheckedState(t *testing.T) { // true -> false emits "false". sb.Set(false) checkbox.JawsUpdate(elem) - tr.InCh <- wire.WsMsg{} if v, ok := waitValue(); !ok || v != "false" { t.Fatalf("expected SetValue %q on true->false, got ok=%v v=%q", "false", ok, v) } @@ -142,7 +140,6 @@ func TestInputBool_JawsUpdateEmitsCheckedState(t *testing.T) { // Unchanged value emits nothing (Last.Swap dedup). checkbox.JawsUpdate(elem) checkbox.JawsUpdate(elem) - tr.InCh <- wire.WsMsg{} if v, ok := waitValue(); ok { t.Fatalf("unchanged bool value re-emitted SetValue %q", v) } diff --git a/lib/ui/multiselect_test.go b/lib/ui/multiselect_test.go index 0ee26170..ca2b0793 100644 --- a/lib/ui/multiselect_test.go +++ b/lib/ui/multiselect_test.go @@ -84,7 +84,6 @@ func TestMultiSelectInitialValues(t *testing.T) { if len(elems) != 1 { t.Fatalf("handler elements = %d, want 1", len(elems)) } - tr.InCh <- wire.WsMsg{} awaitNumberRangeValue(t, tr, elems[0], tt.want) }) } @@ -99,7 +98,6 @@ func TestMultiSelectUpdatesOptionsBeforeValues(t *testing.T) { widget := NewMultiSelect(handler) elem, _ := renderUI(t, tr.Request, widget) before := containerElements(t, elem) - tr.InCh <- wire.WsMsg{} awaitNumberRangeValue(t, tr, elem, `["1","2"]`) handler.children = []jaws.UI{two, three, one} @@ -109,7 +107,6 @@ func TestMultiSelectUpdatesOptionsBeforeValues(t *testing.T) { if len(after) != 3 || after[0] != before[1] || after[2] != before[0] { t.Fatal("reordering did not retain existing option Elements") } - tr.InCh <- wire.WsMsg{} sawAppend, sawOrder := false, false for { select { @@ -171,7 +168,6 @@ func TestMultiSelectInputReconciliation(t *testing.T) { if origin.UI() != peer.UI() || requireContainerState(t, origin) == requireContainerState(t, peer) { t.Fatal("equal MultiSelect definitions must keep separate Element states") } - tr.InCh <- wire.WsMsg{} awaitNumberRangeValue(t, tr, origin, `["1","2"]`) awaitNumberRangeValue(t, tr, peer, `["1","2"]`) diff --git a/lib/ui/number_range_test.go b/lib/ui/number_range_test.go index 39f94e0e..1d4ecfb6 100644 --- a/lib/ui/number_range_test.go +++ b/lib/ui/number_range_test.go @@ -894,7 +894,6 @@ func TestNumericRegisterLogsAndQueuesNothing(t *testing.T) { t.Fatalf("logged errors = %v", logged) } - tr.InCh <- wire.WsMsg{} synctest.Wait() select { case msg := <-tr.OutCh: diff --git a/lib/wire/wsio.go b/lib/wire/wsio.go index 64ee10d6..c3e96580 100644 --- a/lib/wire/wsio.go +++ b/lib/wire/wsio.go @@ -7,6 +7,7 @@ import ( "time" "github.com/coder/websocket" + "github.com/linkdata/jaws/lib/what" ) // writeBatchLimit is the threshold at which WriteLoop stops appending whole @@ -130,6 +131,9 @@ func readWebSocket(ctx context.Context, resultCh chan<- wsReadResult, ws *websoc // WebSocket. // // Consecutive queued records may be coalesced into one text message. +// Reload ends its batch and closes the connection after that batch is written. +// Records queued after Reload are discarded. After closing, ccf(nil) is called +// if ccf is non-nil and neither ctx nor doneCh is done. // // Each WebSocket write has its own writeTimeout deadline; writeTimeout must be // positive. @@ -154,8 +158,15 @@ func WriteLoop(ctx context.Context, ccf context.CancelCauseFunc, doneCh <-chan s return } writectx, writecancel := context.WithTimeout(ctx, writeTimeout) - err = ws.Write(writectx, websocket.MessageText, batchData(msg, outboundMsgCh)) + data, reload := batchData(msg, outboundMsgCh) + err = ws.Write(writectx, websocket.MessageText, data) writecancel() + if err == nil && reload { + // Close before cancelling: a cancelled reader also closes the socket. + _ = ws.Close(websocket.StatusNormalClosure, "") + reportError(ctx, doneCh, ccf, nil) + return + } } } reportError(ctx, doneCh, ccf, err) @@ -185,21 +196,23 @@ func reportError(ctx context.Context, doneCh <-chan struct{}, ccf context.Cancel } } -func batchData(firstMsg WsMsg, outboundMsgCh <-chan WsMsg) []byte { - b := firstMsg.Append(nil) +func batchData(firstMsg WsMsg, outboundMsgCh <-chan WsMsg) (b []byte, reload bool) { + b = firstMsg.Append(nil) + reload = firstMsg.What == what.Reload // accumulate data to send as long as more messages are available until it // exceeds writeBatchLimit batchloop: - for len(b) < writeBatchLimit { + for !reload && len(b) < writeBatchLimit { select { case msg, ok := <-outboundMsgCh: if !ok { break batchloop } b = msg.Append(b) + reload = msg.What == what.Reload default: break batchloop } } - return b + return } diff --git a/lib/wire/wsio_test.go b/lib/wire/wsio_test.go index 9a138635..e4cb1101 100644 --- a/lib/wire/wsio_test.go +++ b/lib/wire/wsio_test.go @@ -2,8 +2,10 @@ package wire import ( "bufio" + "bytes" "context" "errors" + "fmt" "io" "net" "net/http" @@ -681,6 +683,75 @@ func TestWriteLoop_WritesOneFinalFrame(t *testing.T) { waitDone(t, writeDone, "WriteLoop after context cancel") } +func TestWriteLoop_ReloadClosesConnection(t *testing.T) { + for _, preceding := range []string{"", "small", strings.Repeat("x", writeBatchLimit)} { + t.Run(fmt.Sprint(len(preceding)), func(t *testing.T) { + client, server := pipe(t) + client.SetReadLimit(2 * writeBatchLimit) + defer func() { _ = client.CloseNow() }() + defer func() { _ = server.CloseNow() }() + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) + defer cancel() + ioctx, cancelIO := context.WithCancelCause(t.Context()) + defer cancelIO(nil) + inCh := make(chan WsMsg) + readDone := make(chan struct{}) + go func() { + defer close(readDone) + ReadLoop(ioctx, cancelIO, nil, inCh, time.Hour, time.Hour, server) + }() + // After delivering the first record, ReadLoop blocks delivering the + // second. Closing the socket alone cannot unblock that delivery. + input := (&WsMsg{What: what.Input, Jid: 1}).Format() + if err := client.Write(ctx, websocket.MessageText, []byte(input+input)); err != nil { + t.Fatal(err) + } + select { + case <-inCh: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + outCh := make(chan WsMsg, 3) + var want []WsMsg + if preceding != "" { + want = append(want, WsMsg{What: what.Alert, Data: preceding}) + } + want = append(want, WsMsg{What: what.Reload}) + for _, msg := range want { + outCh <- msg + } + outCh <- WsMsg{What: what.Alert, Data: "after reload"} + written := make(chan struct{}) + go func() { + defer close(written) + WriteLoop(ioctx, cancelIO, nil, outCh, time.Second, server) + }() + var got []WsMsg + for { + _, data, err := client.Read(ctx) + if err != nil { + if websocket.CloseStatus(err) != websocket.StatusNormalClosure { + t.Fatal(err) + } + break + } + for record := range bytes.Lines(data) { + msg, ok := Parse(record) + if !ok { + t.Fatalf("invalid record: %q", record) + } + got = append(got, msg) + } + } + waitDone(t, written, "WriteLoop after Reload") + waitDone(t, readDone, "ReadLoop after Reload") + if !slices.Equal(got, want) { + t.Fatalf("messages = %+v, want %+v", got, want) + } + }) + } +} + func TestWriteLoop_RespectsContext(t *testing.T) { outCh := make(chan WsMsg) jawsDoneCh := make(chan struct{}) diff --git a/request.go b/request.go index 66b8e332..e18fb3d3 100644 --- a/request.go +++ b/request.go @@ -90,6 +90,7 @@ type Request struct { Jaws *Jaws // (read-only) the JaWS instance the Request belongs to JawsKey key.Key // (read-only) random key assigned before publication, immutable thereafter; routes JaWS URLs and request-targeted broadcasts only while registered remoteIP netip.Addr // (read-only) remote IP, or the zero netip.Addr if unset + wakeCh chan struct{} // coalesced processing-loop wake-ups; never closed state atomic.Int32 // reqState lifecycle (reqUnclaimable/reqPending/reqClaimed/reqRunning/reqFinished); see loadState/casState lastWriteSeconds atomic.Int32 // [Jaws.runtimeSeconds] value at the most recent RequestWriter write; lock-free, drives pending-eviction preference (pendingEvictionVictimLocked) and idle expiry (maintenance) mu deadlock.RWMutex // protects following @@ -155,7 +156,9 @@ func (s reqState) registered() bool { return s == reqPending || s == reqClaimed // reqClaimed and is not a terminal state). func (s reqState) claimed() bool { return s == reqClaimed || s == reqRunning } -func (rq *Request) loadState() reqState { return reqState(rq.state.Load()) } +func (rq *Request) loadState() reqState { return reqState(rq.state.Load()) } + +// storeState requires an unpublished Request or both rq.mu and jw.mu. func (rq *Request) storeState(s reqState) { rq.state.Store(int32(s)) } func (rq *Request) casState(old, want reqState) bool { return rq.state.CompareAndSwap(int32(old), int32(want)) @@ -308,26 +311,19 @@ func (rq *Request) killSession(wasClaimed bool) { rq.mu.Unlock() } -// deadSession atomically detaches sess and arms one page reload, returning the -// Request identity that belonged to it. -// -// A zero return means rq no longer belongs to sess: it has finished, or has been -// detached from this Session. +// deadSession detaches sess and queues a page reload if rq still belongs to it. // // The reload is queued onto wsQueue rather than broadcast, so a Request whose // WebSocket has not subscribed yet still reloads on connect: process drains // wsQueue before its first select. Holding rq.mu here excludes both a concurrent // recycle and the process loop's getSendMsgs, so the queue append is safe. -// Session.Close wakes an already-running process with a key-targeted Update. -func (rq *Request) deadSession(sess *Session) (k key.Key) { +func (rq *Request) deadSession(sess *Session) { rq.mu.Lock() if rq.session == sess { rq.session = nil - k = rq.JawsKey rq.queue(wire.WsMsg{What: what.Reload}) } rq.mu.Unlock() - return } // sessionDestKey returns rq's identity only while it still belongs to sess. @@ -573,9 +569,10 @@ func (rq *Request) replaceContext(fn func(oldCtx context.Context) (newCtx contex // maintenance performs periodic Request cleanup. // -// It purges browser-removal tombstones. For a request that never went live it -// also cancels and reports expiry once it has been idle (no [RequestWriter] -// write) longer than requestTimeout, or immediately if its context is already +// It wakes queued output and purges browser-removal tombstones. +// For a request that never went live it also cancels and reports expiry once +// it has been idle (no [RequestWriter] write) longer than requestTimeout, or +// immediately if its context is already // done. nowSeconds is the reference instant ([Jaws.runtimeSeconds]). Called from // the Serve loop's maintenance pass while jw.mu is held. // @@ -595,6 +592,12 @@ func (rq *Request) maintenance(nowSeconds int32, requestTimeout time.Duration) ( } } rq.purgeDeletedElementsLocked() + // Maintenance also wakes queued output when no notification is pending. + rq.muQueue.Lock() + if len(rq.wsQueue) > 0 { + rq.wake() + } + rq.muQueue.Unlock() return } @@ -638,6 +641,22 @@ func (rq *Request) Cancel(err error) { rq.cancel(err) } +// Reload queues a command asking the browser to reload its page. +// +// The WebSocket writer sends Reload and closes the connection, which cancels +// the Request's context. Events and callbacks run normally until disconnection. +// A pending Request sends Reload after its [ConnectFn], if any, returns nil. +// Socket writes use the WebSocket timeout; delivery requires a working connection. +// Calling Reload after cancellation has no effect. +// Session data and cookies are unchanged. See [Request] for pointer lifetime. +func (rq *Request) Reload() { + rq.mu.Lock() + if rq.ctx.Err() == nil { + rq.queue(wire.WsMsg{What: what.Reload}) + } + rq.mu.Unlock() +} + // alertData builds the wire payload for an Alert message, HTML-escaping both the // level and the message. Callers may therefore pass untrusted text safely; this // mirrors the escaping done by the internal [wire.WsMsg.FillAlert] path. @@ -884,6 +903,9 @@ func (rq *Request) appendDirtyTags(tags []any) { } rq.todoDirt = append(rq.todoDirt, tagValue) } + if len(rq.todoDirt) > 0 { + rq.wake() + } } rq.mu.Unlock() } @@ -1181,7 +1203,7 @@ func (rq *Request) runWebSocket(ws *websocket.Conn, idleInterval, wsTimeout time // Size the broadcast buffer with headroom that scales with the page's element // count. mustBroadcast (see Jaws.Serve) sends here non-blocking and, if the send // would block, kills the subscription and cancels this request for every message - // except the coalescible nil-destination Update tick, which it drops instead. + // except a coalescible nil-destination Update, which it drops instead. pendingSubscription := rq.Jaws.subscribe(rq, 4+numElems*4) defer func() { // onConnect is user code and may return an error or panic. Release its @@ -1193,11 +1215,7 @@ func (rq *Request) runWebSocket(ws *websocket.Conn, idleInterval, wsTimeout time if err = rq.onConnect(); err == nil { incomingMsgCh := make(chan wire.WsMsg) - // Snapshot ctx after onConnect so a context installed by the callback - // governs all WebSocket loops. - rq.mu.RLock() - ctx := rq.ctx - rq.mu.RUnlock() + ctx := rq.Context() // Transport failures ordinarily only report that the peer is no longer // reachable. A read-limit violation is actionable application feedback, so // retain it even when transport debugging is disabled. @@ -1209,7 +1227,7 @@ func (rq *Request) runWebSocket(ws *websocket.Conn, idleInterval, wsTimeout time } outboundMsgCh := make(chan wire.WsMsg, cap(pendingSubscription)) go wire.ReadLoop(ctx, disconnect, rq.Jaws.Done(), incomingMsgCh, idleInterval, wsTimeout, ws) // closes incomingMsgCh - go wire.WriteLoop(ctx, disconnect, rq.Jaws.Done(), outboundMsgCh, wsTimeout, ws) // calls ws.Close() + go wire.WriteLoop(ctx, disconnect, rq.Jaws.Done(), outboundMsgCh, wsTimeout, ws) // closes the WebSocket broadcastMsgCh := pendingSubscription pendingSubscription = nil // Production deliberately discards the recovered value so a loop panic stays diff --git a/request_reload_test.go b/request_reload_test.go new file mode 100644 index 00000000..59ccb2e0 --- /dev/null +++ b/request_reload_test.go @@ -0,0 +1,146 @@ +package jaws + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/coder/websocket" + "github.com/linkdata/jaws/lib/what" + "github.com/linkdata/jaws/lib/wire" +) + +func TestRequestReload(t *testing.T) { + for _, when := range []string{"pending", "connecting", "connected"} { + t.Run(when, func(t *testing.T) { + jw, err := New() + if err != nil { + t.Fatal(err) + } + go jw.Serve() + defer jw.Close() + hs := httptest.NewServer(jw) + defer hs.Close() + hr := httptest.NewRequest(http.MethodGet, hs.URL+"/", nil) + hr.RemoteAddr = "127.0.0.1:1" + sess := jw.NewSession(httptest.NewRecorder(), hr) + sess.Set("kept", true) + rq := jw.NewRequest(httptest.NewRecorder(), hr) + requestCtx := rq.Context() + connected := make(chan struct{}) + rq.SetConnectFn(func(rq *Request) error { + close(connected) + if when == "connecting" { + rq.Reload() + } + return nil + }) + if when == "pending" { + rq.Reload() + rq.Reload() + } + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) + defer cancel() + conn, _, err := websocket.Dial(ctx, "ws"+strings.TrimPrefix(hs.URL, "http")+"/jaws/"+rq.JawsKeyString(), + &websocket.DialOptions{HTTPHeader: http.Header{"Origin": {hs.URL}}}) + if err != nil { + t.Fatal(err) + } + defer func() { _ = conn.CloseNow() }() + select { + case <-connected: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + if when == "connected" { + rq.Reload() + rq.Reload() + } + writing := make(chan struct{}) + go func() { + defer close(writing) + for conn.Write(ctx, websocket.MessageText, []byte("Input\tJid.1\tignored\n")) == nil { + } + }() + reloads := 0 + for { + _, data, err := conn.Read(ctx) + if err != nil { + if websocket.CloseStatus(err) != websocket.StatusNormalClosure { + t.Fatal(err) + } + break + } + for record := range bytes.Lines(data) { + if msg, ok := wire.Parse(record); ok && msg.What == what.Reload { + reloads++ + } + } + } + if reloads != 1 || sess.Get("kept") != true || sess.Cookie().MaxAge < 0 { + t.Fatalf("reloads=%d, session data=%v, cookie=%v", reloads, sess.Get("kept"), sess.Cookie()) + } + select { + case <-requestCtx.Done(): + case <-ctx.Done(): + t.Fatal("request outlived reload") + } + select { + case <-writing: + case <-ctx.Done(): + t.Fatal("writer outlived reload") + } + }) + } +} + +func TestRequestReloadAllowsEventsUntilDisconnect(t *testing.T) { + rq := newTestRequest(t) + defer rq.Close() + called := make(chan struct{}) + item := &testUi{} + rq.Register(item, func(*Element, string) error { + close(called) + return nil + }) + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) + defer cancel() + rq.Reload() + select { + case msg := <-rq.OutCh: + if msg.What != what.Reload { + t.Fatalf("got %v, want Reload", msg) + } + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + // The harness has no socket writer, so the request remains connected. + select { + case rq.InCh <- wire.WsMsg{Jid: jidForTag(rq.Request, item), What: what.Input}: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + select { + case <-called: + case <-ctx.Done(): + t.Fatal("Reload stopped event dispatch before disconnection") + } +} + +func TestRequestReloadAfterCancel(t *testing.T) { + jw, err := New() + if err != nil { + t.Fatal(err) + } + defer jw.Close() + rq := jw.NewRequest(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil)) + rq.Cancel(nil) + rq.Reload() + if msgs := rq.getSendMsgs(); len(msgs) != 0 { + t.Fatalf("Reload queued messages on a cancelled request: %v", msgs) + } +} diff --git a/request_test.go b/request_test.go index 8a784b58..689ee42a 100644 --- a/request_test.go +++ b/request_test.go @@ -2364,7 +2364,7 @@ func TestRequest_Dirty(t *testing.T) { rq.Dirty(tss1) rq.Dirty(tss2) - // Dirtying marks the elements; the Serve loop broadcasts what.Update only + // Dirtying marks the elements; the Serve loop distributes dirt only // when its updateTicker fires (1ms in tests). Advance the fake clock past // it, then let the process loop re-render both elements (JawsGet again). time.Sleep(2 * time.Millisecond) @@ -2377,8 +2377,8 @@ func TestRequest_Dirty(t *testing.T) { th.True(n1 >= 2) th.True(n2 >= 2) // Pin an upper bound by proving the system quiesces: with jw.dirty now empty, - // distributeDirt returns 0, no further what.Update is broadcast, so getCalled - // must not increase. This catches a runaway re-render/re-broadcast regression. + // distributeDirt has no work and sends no wake-ups, so getCalled + // must not increase. This catches a runaway re-render regression. time.Sleep(2 * time.Millisecond) synctest.Wait() th.Equal(atomic.LoadInt32(&tss1.getCalled), n1) @@ -4420,9 +4420,12 @@ func TestWS_AutoSessionCloseAtPublication(t *testing.T) { readCtx, cancelRead := context.WithTimeout(t.Context(), testTimeout) defer cancelRead() var messages strings.Builder - for !strings.Contains(messages.String(), marker) { + for { messageType, data, err := conn.Read(readCtx) if err != nil { + if websocket.CloseStatus(err) == websocket.StatusNormalClosure { + break + } t.Fatalf("reading WebSocket messages: %v (got %q)", err, messages.String()) } if messageType != websocket.MessageText { @@ -4431,6 +4434,9 @@ func TestWS_AutoSessionCloseAtPublication(t *testing.T) { messages.Write(data) } + if strings.Contains(messages.String(), marker) { + t.Fatal("message sent after Reload") + } if connectSession != nil { t.Errorf("ConnectFn Session() = %v, want nil after Session.Close", connectSession) } diff --git a/request_wake_benchmark_test.go b/request_wake_benchmark_test.go new file mode 100644 index 00000000..5e8b9901 --- /dev/null +++ b/request_wake_benchmark_test.go @@ -0,0 +1,98 @@ +package jaws + +import ( + "strconv" + "testing" + "time" + + "github.com/linkdata/jaws/lib/tag" + "github.com/linkdata/jaws/lib/what" + "github.com/linkdata/jaws/lib/wire" +) + +// BenchmarkRequestQueueDelivery measures queue-to-outbound delivery. +// +// Each parallel worker owns one live Request loop; socket I/O is excluded. +func BenchmarkRequestQueueDelivery(b *testing.B) { + jw := newBenchPoolJaws(b) + jw.updateTicker.Stop() + go jw.ServeWithTimeout(time.Hour) + b.ReportAllocs() + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + rq := NewTestRequest(jw, nil) + if rq == nil { + b.Error("request setup failed") + return + } + defer func() { + rq.Close() + <-rq.DoneCh + }() + <-rq.ReadyCh + msg := wire.WsMsg{What: what.Alert, Data: "queued"} + for pb.Next() { + rq.queue(msg) + if got := <-rq.OutCh; got != msg { + b.Errorf("output = %v, want %v", got, msg) + return + } + } + }) +} + +// BenchmarkRequestMaintenance measures maintenance over idle running Requests. +// +// No transport or processing goroutines compete with the scan. +func BenchmarkRequestMaintenance(b *testing.B) { + for _, n := range []int{100, 1000} { + b.Run("requests="+strconv.Itoa(n), func(b *testing.B) { + jw := newBenchPoolJaws(b) + for range n { + rq := jw.newRequest(nil) + jw.mu.Lock() + rq.mu.Lock() + rq.storeState(reqRunning) + rq.mu.Unlock() + jw.mu.Unlock() + } + b.ReportAllocs() + for b.Loop() { + jw.maintenance(time.Hour) + } + if got := jw.RequestCount(); got != n { + b.Fatalf("request count = %d, want %d", got, n) + } + }) + } +} + +// BenchmarkRequestDirtyFanout measures dirty-tag assignment to pending Requests. +// +// Each iteration sorts and assigns ten tags to 100 Requests, including +// replenishing the dirty set. No process loops run; pending wake signals remain +// coalesced between iterations. +func BenchmarkRequestDirtyFanout(b *testing.B) { + jw := newBenchPoolJaws(b) + reqs := make([]*Request, 100) + for i := range reqs { + reqs[i] = jw.newRequest(nil) + } + tags := make([]any, 10) + for i := range tags { + tags[i] = tag.Tag(strconv.Itoa(i)) + } + b.ReportAllocs() + for b.Loop() { + for _, rq := range reqs { + rq.todoDirt = rq.todoDirt[:0] + } + jw.setDirty(tags) + jw.distributeDirt() + } + for _, rq := range reqs { + if got := len(rq.todoDirt); got != len(tags) { + b.Fatalf("assigned tags = %d, want %d", got, len(tags)) + } + } +} diff --git a/request_wake_test.go b/request_wake_test.go new file mode 100644 index 00000000..de8f628f --- /dev/null +++ b/request_wake_test.go @@ -0,0 +1,80 @@ +package jaws + +import ( + "io" + "testing" + "testing/synctest" + "time" + + "github.com/linkdata/jaws/lib/what" + "github.com/linkdata/jaws/lib/wire" +) + +func TestRequestQueuedWorkWakesIdleLoop(t *testing.T) { + for _, source := range []string{"command", "event", "dirty", "reload", "maintenance"} { + t.Run(source, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + rq := newTestRequest(t) + defer closeRequestInBubble(rq) + rq.Jaws.updateTicker.Stop() + release := make(chan struct{}) + ui := &testUi{ + renderFn: func(elem *Element, _ io.Writer, _ []any) error { + elem.AddHandlers(InputFn(func(elem *Element, value string) error { + <-release + elem.SetValue(value) + return nil + })) + return nil + }, + updateFn: func(elem *Element) { elem.SetValue("queued") }, + } + elem := rq.NewElement(ui) + if err := elem.JawsRender(io.Discard, nil); err != nil { + t.Fatal(err) + } + for range 2 { + synctest.Wait() + want := wire.WsMsg{Jid: elem.Jid(), What: what.Value, Data: "queued"} + switch source { + case "command": + elem.SetValue(want.Data) + case "event": + rq.InCh <- wire.WsMsg{Jid: elem.Jid(), What: what.Input, Data: want.Data} + // The callback waits while the processing loop becomes idle. + synctest.Wait() + release <- struct{}{} + case "dirty": + rq.Dirty(elem) + rq.Jaws.distributeDirt() + case "reload": + want = wire.WsMsg{What: what.Reload} + rq.Reload() + case "maintenance": + // Model queued output without a pending notification so the + // normal maintenance tick must supply the fallback wake-up. + rq.muQueue.Lock() + rq.wsQueue = append(rq.wsQueue, want) + rq.muQueue.Unlock() + synctest.Wait() + select { + case msg := <-rq.OutCh: + t.Fatalf("output before maintenance: %v", msg) + default: + } + time.Sleep(time.Second) + } + synctest.Wait() + select { + case got := <-rq.OutCh: + if got != want { + t.Fatalf("output = %v, want %v", got, want) + } + default: + t.Fatal("queued work did not wake the idle processing loop") + } + } + }) + }) + } +} diff --git a/requestloop.go b/requestloop.go index 361c5689..b2275535 100644 --- a/requestloop.go +++ b/requestloop.go @@ -90,6 +90,8 @@ func (rq *Request) process(broadcastMsgCh chan wire.Message, incomingMsgCh <-cha case <-jawsDoneCh: case <-httpDoneCh: case <-rq.Context().Done(): + case <-rq.wakeCh: + continue case tagmsg, ok = <-broadcastMsgCh: case wsmsg, ok = <-incomingMsgCh: if ok { @@ -234,13 +236,25 @@ func (rq *Request) handleRemove(containerJid Jid, data string) { } } -// queue appends a single outbound message to the request's pending wsQueue under +// wake schedules a processing pass without blocking. One pending signal is enough +// because process drains all queued work before waiting again. +func (rq *Request) wake() { + select { + case rq.wakeCh <- struct{}{}: + default: + } +} + +// queue appends a single outbound message and wakes the processing loop under // muQueue, the leaf lock that orders writes independently of rq.mu. The Request's // process loop drains it through sendQueue and getSendMsgs; drainTailScript drains // tail fixups. func (rq *Request) queue(msg wire.WsMsg) { rq.muQueue.Lock() rq.wsQueue = append(rq.wsQueue, msg) + if len(rq.wsQueue) == 1 { + rq.wake() + } rq.muQueue.Unlock() } diff --git a/requestpool.go b/requestpool.go index b7442f7f..29263e04 100644 --- a/requestpool.go +++ b/requestpool.go @@ -268,6 +268,7 @@ func (jw *Jaws) getRequestLocked(jawsKey key.Key, r *http.Request, remoteIP neti buffers := jw.requestBufferPool.Get().(*requestBuffers) rq = &Request{ Jaws: jw, + wakeCh: make(chan struct{}, 1), buffers: buffers, todoDirt: buffers.todoDirt, elems: buffers.elems, diff --git a/serve.go b/serve.go index 3161cf8e..ac2d2933 100644 --- a/serve.go +++ b/serve.go @@ -107,7 +107,7 @@ func (jw *Jaws) ServeWithTimeout(requestTimeout time.Duration) { } } - // Every addressed frame except the internal Update tick is required, so an + // Every frame except an unaddressed Update is required, so an // overloaded Request is cancelled instead of silently losing a frame. mustBroadcast := func(msg wire.Message) { for msgCh, rq := range subs { @@ -117,7 +117,7 @@ func (jw *Jaws) ServeWithTimeout(requestTimeout time.Duration) { select { case msgCh <- msg: default: - // Only an unaddressed Update tick can be dropped. + // Only an unaddressed Update can be dropped. if msg.What != what.Update || msg.Dest != nil { killSub(msgCh) rq.cancel(fmt.Errorf("%w: %v: broadcast channel full sending %s", ErrRequestOverloaded, rq, msg.String())) @@ -131,9 +131,7 @@ func (jw *Jaws) ServeWithTimeout(requestTimeout time.Duration) { normalShutdown = true return case <-jw.updateTicker.C: - if jw.distributeDirt() > 0 { - mustBroadcast(wire.Message{What: what.Update}) - } + jw.distributeDirt() case <-t.C: jw.refreshRuntimeSeconds() jw.maintenance(requestTimeout) diff --git a/session.go b/session.go index 07ecdb5b..29afd899 100644 --- a/session.go +++ b/session.go @@ -219,12 +219,9 @@ func (sess *Session) addCookie(w http.ResponseWriter, r *http.Request) (added bo // Existing [Request] values already associated with the [Session] will ask the // browser to reload the pages. This holds even for a [Request] whose WebSocket // has not connected yet: the reload is queued on the [Request] and delivered when -// it connects. +// it connects. Each WebSocket closes after sending Reload. // Key/value pairs in the [Session] are left unmodified; use [Session.Clear] to remove all of them. // -// It must not be called before the JaWS processing loop ([Jaws.Serve] or -// [Jaws.ServeWithTimeout]) is running, because the wake-up broadcasts may block. -// // Close returns a non-nil deletion cookie for a non-nil [Session]. func (sess *Session) Close() (cookie *http.Cookie) { if sess != nil { @@ -238,23 +235,16 @@ func (sess *Session) Close() (cookie *http.Cookie) { *cookie = sess.cookie sess.mu.Unlock() - // deadSession queues the reload directly onto each Request, covering those - // whose WebSocket has not subscribed yet. This key-targeted Update is only a - // wake-up: it makes an already-running process loop iterate and flush the - // queued reload. handleBroadcast resolves a key destination to no elements, - // so the Update itself performs no browser operation. - msg := wire.Message{What: what.Update} for _, rq := range requests { - if k := rq.deadSession(sess); k != 0 { - msg.Dest = k - sess.jw.Broadcast(msg) - } + rq.deadSession(sess) } } return } -// Reload calls [Session.Broadcast] with a message asking browsers to reload the page. +// Reload asks the Session's active pages to reload. +// +// Each WebSocket closes after sending Reload. // See [Session.Broadcast] for the processing-loop requirement. func (sess *Session) Reload() { sess.Broadcast(wire.Message{What: what.Reload}) diff --git a/session_test.go b/session_test.go index 66eaf965..187489cd 100644 --- a/session_test.go +++ b/session_test.go @@ -1289,17 +1289,8 @@ func TestSession_ProducersSkipRecycled(t *testing.T) { default: } - // Session.Close arms a reload on the live request only, and wakes it with a - // key-targeted Update. The finished request gets neither. - closeDone := make(chan struct{}) - go func() { - sess.Close() - close(closeDone) - }() - got = nextBroadcast(t, jw) - th.Equal(got.What, what.Update) - th.Equal(got.Dest, live.JawsKey) - // deadSession queued the reload before the wake broadcast, so it is visible now. + // Session.Close queues a reload only on the live Request, without broadcasting. + sess.Close() live.muQueue.Lock() th.Equal(len(live.wsQueue), 1) if len(live.wsQueue) == 1 { @@ -1311,11 +1302,6 @@ func TestSession_ProducersSkipRecycled(t *testing.T) { th.Equal(len(finished.wsQueue), 0) finished.muQueue.Unlock() select { - case <-closeDone: - case <-time.After(time.Second): - t.Fatal("timeout waiting for Session.Close") - } - select { case extra := <-jw.bcastCh: t.Fatalf("recycled request must not broadcast, got %#v", extra) default: @@ -1458,8 +1444,7 @@ func TestSessionCloseDoesNotReachLaterRequest(t *testing.T) { // TestSessionCloseReloadsAssociatedPendingRequest covers issue #215: closing a // Session must reload a Request that is associated but whose WebSocket has not // subscribed yet. The reload is queued on the Request by Session.Close and -// delivered when the WebSocket connects, independent of the (dropped) wake-up -// broadcast. +// delivered when the WebSocket connects. func TestSessionCloseReloadsAssociatedPendingRequest(t *testing.T) { jw, err := New() if err != nil { @@ -1487,37 +1472,8 @@ func TestSessionCloseReloadsAssociatedPendingRequest(t *testing.T) { t.Fatalf("target state = %v, want %v", got, reqPending) } - // A separate control subscription, not attached to the session, is an ordering - // probe against the serve loop: broadcasts are processed in order. - controlHTTP := httptest.NewRequest(http.MethodGet, server.URL+"/", nil) - controlHTTP.RemoteAddr = "127.0.0.2:1" - control := jw.newRequest(controlHTTP) - if control.Session() != nil { - t.Fatal("control Request must not share the session") - } - controlCh := jw.subscribe(control, 8) - if controlCh == nil { - t.Fatal("control subscription failed") - } - waitForServeLoop(t, jw) // ensure control is installed in subs - sess.Close() - // Prove the close wake-up was processed while the target had no subscription: - // once the control marker arrives, the earlier key-targeted Update to the - // (unsubscribed) target has already been handled and dropped. The unfixed - // implementation broadcast the reload here and lost it. - const controlMarker = "control ordering marker" - jw.Broadcast(wire.Message{Dest: control.JawsKey, What: what.Alert, Data: controlMarker}) - select { - case msg := <-controlCh: - if msg.What != what.Alert || msg.Data != controlMarker { - t.Fatalf("control subscription got %#v, want Alert %q", msg, controlMarker) - } - case <-time.After(time.Second): - t.Fatal("timeout waiting for control ordering marker") - } - // Now the target's browser opens its WebSocket. connected := make(chan struct{}) target.SetConnectFn(func(*Request) error { @@ -1545,18 +1501,19 @@ func TestSessionCloseReloadsAssociatedPendingRequest(t *testing.T) { t.Fatal("target Request did not start its WebSocket") } - // A post-connect marker terminates the read loop. Batching is opportunistic, so - // the reload may arrive in an earlier frame; accumulate frames until the marker - // and assert exactly one Reload command survived the close. + // Reload terminates the connection; messages queued after it are discarded. const targetMarker = "post-connect marker" jw.Broadcast(wire.Message{Dest: target.JawsKey, What: what.Alert, Data: targetMarker}) readCtx, cancelRead := context.WithTimeout(t.Context(), 3*time.Second) defer cancelRead() var acc strings.Builder - for !strings.Contains(acc.String(), targetMarker) { + for { mt, data, err := conn.Read(readCtx) if err != nil { + if websocket.CloseStatus(err) == websocket.StatusNormalClosure { + break + } t.Fatalf("reading from target Request: %v (got %q)", err, acc.String()) } if mt != websocket.MessageText { @@ -1564,6 +1521,9 @@ func TestSessionCloseReloadsAssociatedPendingRequest(t *testing.T) { } acc.Write(data) } + if strings.Contains(acc.String(), targetMarker) { + t.Fatal("message sent after Reload") + } if n := strings.Count(acc.String(), what.Reload.String()+"\t"); n != 1 { t.Fatalf("got %d Reload commands, want exactly 1: %q", n, acc.String()) } @@ -1571,8 +1531,7 @@ func TestSessionCloseReloadsAssociatedPendingRequest(t *testing.T) { // TestSessionCloseReloadsConnectedRequestExactlyOnce closes a Session whose // Request is already connected and asserts that exactly one Reload is delivered. -// It reads through a post-close marker rather than a single frame, so a delayed -// duplicate reload would be caught. +// It reads until normal closure and checks that messages after Reload are discarded. func TestSessionCloseReloadsConnectedRequestExactlyOnce(t *testing.T) { jw, err := New() if err != nil { @@ -1620,8 +1579,7 @@ func TestSessionCloseReloadsConnectedRequestExactlyOnce(t *testing.T) { t.Fatal("Request did not start its WebSocket") } - // The Request is now connected and subscribed; close the session and then send - // a marker to bound the read. + // Close the session and queue a marker that must be discarded after Reload. sess.Close() const marker = "post-close marker" jw.Broadcast(wire.Message{Dest: rq.JawsKey, What: what.Alert, Data: marker}) @@ -1629,9 +1587,12 @@ func TestSessionCloseReloadsConnectedRequestExactlyOnce(t *testing.T) { readCtx, cancelRead := context.WithTimeout(t.Context(), 3*time.Second) defer cancelRead() var acc strings.Builder - for !strings.Contains(acc.String(), marker) { + for { mt, data, err := conn.Read(readCtx) if err != nil { + if websocket.CloseStatus(err) == websocket.StatusNormalClosure { + break + } t.Fatalf("reading from Request: %v (got %q)", err, acc.String()) } if mt != websocket.MessageText { @@ -1639,15 +1600,16 @@ func TestSessionCloseReloadsConnectedRequestExactlyOnce(t *testing.T) { } acc.Write(data) } + if strings.Contains(acc.String(), marker) { + t.Fatal("message sent after Reload") + } if n := strings.Count(acc.String(), what.Reload.String()+"\t"); n != 1 { t.Fatalf("got %d Reload commands, want exactly 1: %q", n, acc.String()) } } -// TestServeKeyTargetedUpdateFailFast verifies the overload classification the -// Session.Close wake-up relies on: only the internal nil-destination Update tick -// is droppable, while every addressed Update — tag-targeted or key-targeted — is -// one-shot and must fail-fast an overloaded Request. +// TestServeKeyTargetedUpdateFailFast verifies that an unaddressed Update is +// coalescible, while every addressed Update must fail-fast an overloaded Request. func TestServeKeyTargetedUpdateFailFast(t *testing.T) { jw, err := New() if err != nil { @@ -1694,7 +1656,7 @@ func TestServeKeyTargetedUpdateFailFast(t *testing.T) { } } - // A nil-destination Update is the coalescible dirty-render tick: overflowing it + // A nil-destination Update is coalescible: overflowing it // must neither cancel the Request nor kill its subscription. dropRq := jw.newRequest(httptest.NewRequest(http.MethodGet, "/", nil)) dropCh := jw.subscribe(dropRq, 1) // not drained during the overflow below @@ -1748,7 +1710,7 @@ func TestServeKeyTargetedUpdateFailFast(t *testing.T) { } awaitCancel("tag-targeted Update", tagRq) - // A key-targeted Update is the Session.Close wake-up: overflow must fail-fast. + // A key-targeted Update is addressed: overflow must fail-fast. wakeRq := jw.newRequest(httptest.NewRequest(http.MethodGet, "/", nil)) if jw.subscribe(wakeRq, 1) == nil { // never drained t.Fatal("wake subscription failed") diff --git a/status_test.go b/status_test.go index a0af0070..03c3d565 100644 --- a/status_test.go +++ b/status_test.go @@ -15,8 +15,6 @@ import ( "time" "github.com/coder/websocket" - "github.com/linkdata/jaws/lib/what" - "github.com/linkdata/jaws/lib/wire" ) func newStatusTestJaws(t *testing.T, configure func(*Jaws)) *Jaws { @@ -157,7 +155,6 @@ func dispatchStatusDirt(t *testing.T, jw *Jaws, want int) { if got := jw.distributeDirt(); got != want { t.Fatalf("distributed status tags = %d, want %d", got, want) } - jw.Broadcast(wire.Message{What: what.Update}) } func requireStatusCount(t *testing.T, values <-chan int, want int) {