diff --git a/src/main/character-dictionary-runtime/cache.test.ts b/src/main/character-dictionary-runtime/cache.test.ts index 6014f330..36c8c740 100644 --- a/src/main/character-dictionary-runtime/cache.test.ts +++ b/src/main/character-dictionary-runtime/cache.test.ts @@ -39,6 +39,46 @@ test('writeSnapshot persists and readSnapshot restores current-format snapshots' assert.deepEqual(await readSnapshot(snapshotPath), { ...snapshot, nameSplitSource: 'heuristic' }); }); +// A manual generate and an auto-sync can both land on the same media, so two writes for one +// snapshot can overlap. They must not stream into a shared temp file and interleave into a +// half-and-half snapshot. +test('concurrent writeSnapshot calls for the same media leave one complete snapshot', async () => { + const outputDir = makeTempDir(); + const snapshotPath = getSnapshotPath(outputDir, 130298); + const base = createSnapshot(); + const wide: CharacterDictionarySnapshot = { + ...base, + entryCount: 400, + termEntries: Array.from({ length: 400 }, (_entry, index) => [ + `名前${index}`, + 'なまえ', + 'name primary', + '', + 75, + [`Character ${index}`.repeat(200)], + 0, + '', + ]) as CharacterDictionarySnapshot['termEntries'], + }; + + await Promise.all([ + writeSnapshot(snapshotPath, wide), + writeSnapshot(snapshotPath, wide), + writeSnapshot(snapshotPath, wide), + ]); + + const restored = await readSnapshot(snapshotPath); + assert.equal(restored?.entryCount, 400); + assert.equal(restored?.termEntries.length, 400); + assert.deepEqual(restored, { ...wide, nameSplitSource: 'heuristic' }); + + // Every writer cleaned up after itself, so no temp files are left behind. + const leftovers = fs + .readdirSync(path.dirname(snapshotPath)) + .filter((name) => name.includes('.tmp-')); + assert.deepEqual(leftovers, []); +}); + test('readSnapshot preserves the mecab name-split source and defaults missing values to heuristic', async () => { const outputDir = makeTempDir(); const snapshotPath = getSnapshotPath(outputDir, 130298); diff --git a/src/main/character-dictionary-runtime/cache.ts b/src/main/character-dictionary-runtime/cache.ts index be8ecd18..e392f0ff 100644 --- a/src/main/character-dictionary-runtime/cache.ts +++ b/src/main/character-dictionary-runtime/cache.ts @@ -172,6 +172,10 @@ export async function readSnapshot( // JSON.stringify of a large snapshot blocks the event loop for seconds. const SNAPSHOT_WRITE_FLUSH_BYTES = 4 * 1024 * 1024; +// Distinguishes concurrent writes of the same snapshot within one process; the pid alone only +// separates processes, so two overlapping writers would otherwise stream into the same temp file. +let snapshotWriteSequence = 0; + /** * Streams the snapshot to disk piece by piece instead of stringifying it in one shot, then renames * the finished file into place so a crash mid-write (or two concurrent writers for the same media) @@ -182,7 +186,8 @@ export async function writeSnapshot( snapshot: CharacterDictionarySnapshot, ): Promise { ensureDir(path.dirname(snapshotPath)); - const tempPath = `${snapshotPath}.tmp-${process.pid}`; + snapshotWriteSequence += 1; + const tempPath = `${snapshotPath}.tmp-${process.pid}-${snapshotWriteSequence}`; const handle = await fs.promises.open(tempPath, 'w'); try { let buffered: string[] = []; diff --git a/src/main/character-dictionary-runtime/zip.ts b/src/main/character-dictionary-runtime/zip.ts index 9ab99604..1051a059 100644 --- a/src/main/character-dictionary-runtime/zip.ts +++ b/src/main/character-dictionary-runtime/zip.ts @@ -78,7 +78,11 @@ export async function buildDictionaryZip( }; } - const entriesPerBank = 10_000; + // Each bank is stringified in one shot, so the bank size sets the longest single block in the + // build. 10k entries measured ~38MB and ~135ms per bank on a real merged dictionary; 2k keeps + // every bank under the archive writer's yield budget at ~27ms. Yomitan reads any number of + // term_bank_N.json files, so this only changes how the terms are split across them. + const entriesPerBank = 2_000; for (let i = 0; i < termEntries.length; i += entriesPerBank) { yield { name: `term_bank_${Math.floor(i / entriesPerBank) + 1}.json`, diff --git a/src/shared/stored-zip.test.ts b/src/shared/stored-zip.test.ts new file mode 100644 index 00000000..7dd2f1eb --- /dev/null +++ b/src/shared/stored-zip.test.ts @@ -0,0 +1,74 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import test from 'node:test'; +import { readStoredZipFirstFile, writeStoredZip, writeStoredZipAsync } from './stored-zip'; + +function makeTempDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-stored-zip-')); +} + +function readEntries(zipPath: string): Map { + const archive = fs.readFileSync(zipPath); + const entries = new Map(); + let cursor = 0; + + while (cursor + 4 <= archive.length) { + const signature = archive.readUInt32LE(cursor); + if (signature === 0x02014b50 || signature === 0x06054b50) { + break; + } + assert.equal(signature, 0x04034b50, `unexpected local file header at offset ${cursor}`); + const size = archive.readUInt32LE(cursor + 18); + const nameLength = archive.readUInt16LE(cursor + 26); + const extraLength = archive.readUInt16LE(cursor + 28); + const nameStart = cursor + 30; + const dataStart = nameStart + nameLength + extraLength; + entries.set( + archive.subarray(nameStart, nameStart + nameLength).toString('utf8'), + Buffer.from(archive.subarray(dataStart, dataStart + size)), + ); + cursor = dataStart + size; + } + + return entries; +} + +// The async writer yields on a byte budget between entries, so an entry that is itself larger than +// that budget is the case where the accounting could drift: the offsets, CRCs, and central +// directory all have to come out identical to the synchronous writer. +test('writeStoredZipAsync writes a correct archive when one entry exceeds the yield budget', async () => { + const dir = makeTempDir(); + try { + // Comfortably past the writer's 8MB yield budget. + const oversized = Buffer.alloc(10 * 1024 * 1024); + for (let i = 0; i < oversized.length; i += 1) { + oversized[i] = i % 251; + } + const files = [ + { name: 'index.json', data: Buffer.from('{"revision":"rev-1"}', 'utf8') }, + { name: 'big.bin', data: oversized }, + { name: 'after.txt', data: Buffer.from('written after the oversized entry', 'utf8') }, + ]; + + const asyncPath = path.join(dir, 'async.zip'); + const syncPath = path.join(dir, 'sync.zip'); + const asyncResult = await writeStoredZipAsync(asyncPath, files); + const syncResult = writeStoredZip(syncPath, files); + + assert.equal(asyncResult.entryCount, 3); + assert.deepEqual(asyncResult, syncResult); + // Byte-identical to the synchronous writer: yielding mid-archive changed no offset or CRC. + assert.ok(fs.readFileSync(asyncPath).equals(fs.readFileSync(syncPath))); + + const entries = readEntries(asyncPath); + assert.deepEqual([...entries.keys()], ['index.json', 'big.bin', 'after.txt']); + assert.ok(entries.get('big.bin')!.equals(oversized)); + assert.equal(entries.get('after.txt')!.toString('utf8'), 'written after the oversized entry'); + // The trailing records still parse, which is what proves the archive is complete. + assert.equal(readStoredZipFirstFile(asyncPath)?.name, 'index.json'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +});