Compare commits

..
Author SHA1 Message Date
logan 49445ded77 test(download): write the yt-dlp stub under ForkLock
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m48s
CI / e2e (pull_request) Successful in 10m21s
The kernel refuses to exec a file that is open for writing anywhere in
the process, and these tests are parallel: a sibling's fork duplicates
stubYtDlp's write descriptor in the moment it is open and carries it
past our close, so the exec a moment later fails with ETXTBSY. That is
the flake seen once locally and once in CI, both times on a tree with
no Go in its diff.

Closing sooner is not available -- os.WriteFile has already closed the
file before anything execs it -- and O_CLOEXEC does not help, because
the window is between another goroutine's fork and its own exec.
syscall.ForkLock is the lock forkExec takes across that fork, so
holding it over the write means no child can exist while the
descriptor does.

Measured on the helper itself under 12 concurrent writers: 176-189 of
2400 execs refused before, 0 of 2400 after, three runs each.

Closes #146
2026-08-30 07:40:32 -04:00
3 changed files with 26 additions and 168 deletions
+21 -3
View File
@@ -7,6 +7,7 @@ import (
"path/filepath"
"runtime"
"strings"
"syscall"
"testing"
)
@@ -17,6 +18,21 @@ import (
// stubYtDlp writes an executable script that echoes the given stdout
// and returns it as a provider config binary path.
//
// The write is held under syscall.ForkLock, and that is not tidiness:
// the kernel refuses to exec a file that is open for writing anywhere
// in the process, and these tests are parallel, so a *sibling* test's
// fork can duplicate this descriptor in the moment it is open and
// carry it past our close — the exec a moment later then fails with
// ETXTBSY, "text file busy". That is #146, seen once in CI and once
// locally, on trees containing no Go at all. Closing sooner is not
// available (os.WriteFile has already closed the file before anything
// execs it) and O_CLOEXEC does not help, because the window is between
// another goroutine's fork and its own exec. ForkLock is the lock
// syscall.forkExec takes across that fork, so holding it here means no
// child can exist while the descriptor does. Measured on this helper
// under 12 concurrent writers: 176-189 of 2400 execs refused without
// it, 0 of 2400 with it.
func stubYtDlp(t *testing.T, script string) string {
t.Helper()
@@ -26,9 +42,11 @@ func stubYtDlp(t *testing.T, script string) string {
path := filepath.Join(t.TempDir(), "yt-dlp")
if err := os.WriteFile(
path, []byte("#!/bin/sh\n"+script), 0o700,
); err != nil {
syscall.ForkLock.Lock()
err := os.WriteFile(path, []byte("#!/bin/sh\n"+script), 0o700)
syscall.ForkLock.Unlock()
if err != nil {
t.Fatalf("write stub: %v", err)
}
@@ -2,14 +2,12 @@ import { LitElement, html, css, nothing } from 'lit';
import { customElement, state, query } from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import { EventsOn } from '@runtime/runtime';
import {
AddLibrary,
GetAllLibrariesWithTrackCounts,
} from '@go/library/library.js';
import { describeError, explainError } from '@utils/describe-error';
import { nameDialogsIn } from '@utils/name-dialog';
import { Events } from '../../events';
import { pickDirectory } from '../../utils/pick-directory';
/**
@@ -21,13 +19,6 @@ import { pickDirectory } from '../../utils/pick-directory';
* prompting the user to pick their music folder, registers it through the
* library CRUD API, and dismisses itself. AddLibrary emits LibraryAdded
* and kicks off the initial scan automatically.
*
* **The dismissal follows the library existing, not the button being
* pressed.** `AddLibrary` emits `LibraryAdded` whoever calls it, so the
* wizard waits on the state it exists to wait for rather than on a step
* in its own flow — a library arriving by any other route (Settings, a
* direct call) leaves a full-screen modal up otherwise, intercepting
* every pointer event.
*/
@customElement('first-run-wizard')
export class FirstRunWizard extends LitElement {
@@ -46,18 +37,9 @@ export class FirstRunWizard extends LitElement {
/** Error message from a failed pick/save, if any. */
@state() private errorMessage = '';
/** Unsubscribe from LibraryAdded, while this element is connected. */
private cancelLibraryAdded?: () => void;
override async connectedCallback(): Promise<void> {
super.connectedCallback();
// Subscribed before the read below, so a library arriving while
// that call is in flight is not answered with a stale empty list.
this.cancelLibraryAdded = EventsOn(Events.LibraryAdded, () => {
this.dismiss();
});
try {
const existing = await GetAllLibrariesWithTrackCounts();
@@ -72,8 +54,6 @@ export class FirstRunWizard extends LitElement {
return;
}
if (this.finished) return;
this.active = true;
await this.updateComplete;
@@ -81,13 +61,6 @@ export class FirstRunWizard extends LitElement {
if (this.dialog) this.dialog.open = true;
}
override disconnectedCallback(): void {
this.cancelLibraryAdded?.();
this.cancelLibraryAdded = undefined;
super.disconnectedCallback();
}
static override styles = css`
wa-dialog {
--width: 480px;
@@ -266,20 +239,6 @@ export class FirstRunWizard extends LitElement {
if (!this.finished) e.preventDefault();
};
/**
* Close, and stay closed: a library exists, so setup is over.
*
* `finished` is set first, or `preventClose` cancels the hide this
* asks for.
*/
private dismiss(): void {
this.finished = true;
if (this.dialog) this.dialog.open = false;
this.active = false;
}
private handleChoose = async (): Promise<void> => {
this.errorMessage = '';
@@ -305,7 +264,11 @@ export class FirstRunWizard extends LitElement {
try {
await AddLibrary(this.selectedDirectory);
this.dismiss();
this.finished = true;
if (this.dialog) this.dialog.open = false;
this.active = false;
} catch (err) {
this.errorMessage = explainError(
err,
@@ -1,123 +0,0 @@
/**
* #175: the first-run wizard's dismissal follows the library existing,
* not its own button being pressed.
*
* The wizard is a modal that blocks every pointer event, so a library
* arriving by another route — Settings, a direct call — used to leave
* it up over an app that was already set up. `LibraryAdded` is emitted
* by `AddLibrary` whoever calls it, which is what makes one
* subscription the whole fix.
*/
import { beforeEach, describe, expect, it } from 'vitest';
import { Events } from '../../src/events';
import { emit, stub } from '../support/harness';
import { fixture, shadow, shadowAll } from '../support/render';
import { wails } from '../support/wails-fake';
import '@components/first-run-wizard/first-run-wizard';
import type { FirstRunWizard } from '@components/first-run-wizard/first-run-wizard';
/** A library row, as `GetAllLibrariesWithTrackCounts` returns one. */
const aLibrary = {
id: 1,
name: 'Music',
path: '/home/logan/Music',
trackCount: 9,
};
/** Mount the wizard on a fresh install: no libraries yet. */
async function wizardOnAFreshInstall(): Promise<FirstRunWizard> {
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
return fixture<FirstRunWizard>('first-run-wizard');
}
/** Whether the wizard is rendering its modal at all. */
function isShowing(el: FirstRunWizard): boolean {
return shadow(el, 'wa-dialog') !== null;
}
beforeEach(() => {
stub('library.Library.AddLibrary', aLibrary);
});
describe('first-run-wizard', () => {
it('shows on a fresh install and stays up until a library exists', async () => {
const el = await wizardOnAFreshInstall();
expect(isShowing(el)).toBe(true);
});
it('stays hidden when a library is already configured', async () => {
stub('library.Library.GetAllLibrariesWithTrackCounts', [aLibrary]);
const el = await fixture<FirstRunWizard>('first-run-wizard');
expect(isShowing(el)).toBe(false);
});
it('dismisses when a library appears by another route', async () => {
const el = await wizardOnAFreshInstall();
expect(isShowing(el)).toBe(true);
emit(Events.LibraryAdded, aLibrary);
await el.updateComplete;
expect(isShowing(el)).toBe(false);
});
it('does not raise itself when a library arrives while it is asking', async () => {
// The read is still in flight when the event lands, so its
// answer — an empty list — is stale by the time it returns.
let answer: (libraries: unknown[]) => void = () => {};
stub(
'library.Library.GetAllLibrariesWithTrackCounts',
() =>
new Promise((resolve) => {
answer = resolve;
}),
);
const el = await fixture<FirstRunWizard>('first-run-wizard');
emit(Events.LibraryAdded, aLibrary);
answer([]);
await el.updateComplete;
await new Promise((r) => setTimeout(r, 0));
await el.updateComplete;
expect(isShowing(el)).toBe(false);
});
it('still dismisses through its own Get Started button', async () => {
stub('frontendutil.FrontendUtil.HasNativeDirectoryPicker', true);
stub('frontendutil.FrontendUtil.DirectoryPicker', '/home/logan/Music');
const { resetDirectoryPickerCache } = await import(
'@utils/pick-directory'
);
resetDirectoryPickerCache();
const el = await wizardOnAFreshInstall();
const [choose, finish] = shadowAll<HTMLButtonElement>(el, '.btn');
choose?.click();
await new Promise((r) => setTimeout(r, 0));
await el.updateComplete;
finish?.click();
await new Promise((r) => setTimeout(r, 0));
await el.updateComplete;
expect(
wails.calls.filter((c) => c.path === 'library.Library.AddLibrary'),
).toHaveLength(1);
expect(isShowing(el)).toBe(false);
});
});