rune: displays made while the tab is still loading are delivered twice, out of order

[martin-henz]

Summary

On conductor-migration, rune delivers a display twice, and out of order, when a program draws more than once while the tab is still loading. show(square); anaglyph(circle); renders three canvases instead of two.

This was found while migrating csg (which was about to inherit the same pattern, copied from rune) and is confirmed with an end-to-end repro against the real Channel.

Cause

Two delivery paths overlap in src/bundles/rune/src/index.ts.

__display streams live as soon as the tab has been asked to load:

private async __display(message: RuneDisplayMessage): Promise<void> {
  this.__displayed.push(message);
  if (this.__loadRuneTab()) {          // false only on the very first call
    this.__runeChannel.send(message);
  }
}

and the constructor replays the whole backlog when the tab asks for it:

this.__runeChannel.subscribe(message => {
  if (message.type === 'request') {
    this.__displayed.forEach(m => this.__runeChannel.send(m));
  }
});

__loadRuneTab() sets __tabLoaded = true and returns false on the first display only; every later display streams live. But loadTab() only starts loading the tab — fetching build/tabs/Rune.js is asynchronous, and RuneTabPlugin doesn't subscribe and send { type: 'request' } until it has been constructed. So every display between the first one and the tab actually loading is sent live and replayed.

Those live sends are not dropped. Channel buffers messages that arrive before anyone has subscribed, and flushes them to the first subscriber (dist/Conduit-*.js, buffer capped at SETUP_MESSAGES_BUFFER_SIZE = 10):

_(msg) {
  if (this.o) {                     // no subscribers yet -> buffer
    if (this.o.length >= SETUP_MESSAGES_BUFFER_SIZE) return console.warn(...);
    this.o.push(msg);
  } else for (const sub of this.i) sub(msg);
}
subscribe(sub) {
  this.i.add(sub);
  if (this.o) { for (const m of this.o) sub(m); delete this.o; }   // flush
}

So RuneTabPlugin's subscribe receives the buffered live sends before it sends request — which is also why the duplicate arrives first and the ordering is wrong.

Reproduction

Drop this in src/bundles/rune/src/__tests__/ and run yarn workspace @sourceacademy/bundle-rune test:

import { Channel } from '@sourceacademy/conductor/conduit';
import { DataType } from '@sourceacademy/conductor/types';
import { expect, test } from 'vitest';
import RuneModulePlugin from '..';
import { RUNE_CHANNEL_ID, type RuneChannelMessage } from '../protocol';

function makeEvaluator() {
  const store: unknown[] = [];
  return {
    hasDataInterface: true as const,
    closure_make: async (sig: unknown, func: unknown) => ({ type: DataType.CLOSURE, value: { sig, func } }),
    opaque_make: async (value: unknown) => {
      store.push(value);
      return { type: DataType.OPAQUE, value: store.length - 1 };
    },
    opaque_get: async (tv: { value: number }) => store[tv.value]
  };
}

test('rune delivers a display twice when it happens while the tab is still loading', async () => {
  const { port1, port2 } = new MessageChannel();
  const runnerChannel = new Channel<RuneChannelMessage>(RUNE_CHANNEL_ID, port1 as any);
  const webChannel = new Channel<RuneChannelMessage>(RUNE_CHANNEL_ID, port2 as any);

  // Loading a tab means fetching build/tabs/Rune.js, so the tab plugin does not
  // exist until some time after loadTab() returns.
  const tabLoader = { tabs: ['Rune'], loadTab: () => {} };

  const evaluator = makeEvaluator();
  const plugin = new RuneModulePlugin({} as any, [runnerChannel] as any, evaluator as any, tabLoader);
  await plugin.initialise();

  const square = plugin.exports.find(e => e.symbol === 'square')!.value as any;
  const circle = plugin.exports.find(e => e.symbol === 'circle')!.value as any;

  for await (const _ of plugin.show(square)) { /* drive */ }
  for await (const _ of plugin.anaglyph(circle)) { /* drive */ }

  // The tab finally loads and does what RuneTabPlugin's constructor does.
  await new Promise(resolve => setTimeout(resolve, 50));
  const received: RuneChannelMessage[] = [];
  webChannel.subscribe(message => received.push(message));
  webChannel.send({ type: 'request' });
  await new Promise(resolve => setTimeout(resolve, 50));

  const renders = received.filter(m => m.type === 'render');
  console.log(renders.map((r: any) => r.mode));   // [ 'anaglyph', 'normal', 'anaglyph' ]
  expect(renders).toHaveLength(2);                // actual: 3
});

Expected: 2 renders, ['normal', 'anaglyph'].
Actual: 3 renders, ['anaglyph', 'normal', 'anaglyph'].

Since MultiItemDisplay renders one item per message, the user sees three canvases, with the second drawing appearing first.

Suggested fix

Don't stream until the tab has actually asked. Queue everything until the first request, then send the backlog and switch to live:

private __tabRequested = false;

// in the constructor's subscriber
if (message.type === 'request') {
  this.__tabRequested = true;
  this.__displayed.forEach(m => this.__runeChannel.send(m));
}

private async __display(message: RuneDisplayMessage): Promise<void> {
  this.__displayed.push(message);
  this.__loadRuneTab();               // no longer needs a boolean return
  if (this.__tabRequested) {
    this.__runeChannel.send(message);
  }
}

This gives exactly-once delivery in the normal flow, and still replays correctly if the tab is ever recreated (a fresh tab starts with an empty message list, so a full replay is what it wants).

This is what csg does in its Conductor migration; it is covered there by the test renders made before the tab asks are replayed once, not duplicated.

Not affected

• sound — no replay buffer and no request handshake; it drives the tab entirely through makeRpc, so there is no fire-and-forget path that could double-deliver.
• Rune is the only other module using this pattern (grep -rn "'request'" src/bundles/*/src/*.ts).

_{The repro above runs against rune only because rune uses attachModuleMethod; the equivalent test can't currently be written for a @moduleMethod-decorated plugin index (#816).}

[11suixing11]

Hi, I can take this one.

I will keep it scoped to rune display delivery: add a regression test around the request/replay handshake using the real Channel, then change the module to queue displays until the Rune tab has actually requested the backlog.

[martin-henz]

Thanks a lot @11suixing11 for contributing.