Skip to content

Commit b1f222e

Browse files
authored
Merge pull request #5 from rodrigogs/copilot/sub-pr-4
fix: prevent worker timeout leaks and scroll race conditions
2 parents c1b5e5a + faf2580 commit b1f222e

4 files changed

Lines changed: 98 additions & 46 deletions

File tree

src/lib/components/ChatView.svelte

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -343,32 +343,49 @@ $effect(() => {
343343
const maxRetries = 20; // 20 * 50ms = 1 second max
344344
const retryDelay = 50;
345345
346+
// Capture targetId at the start to detect if it changes during retries
347+
const capturedTargetId = targetId;
348+
346349
for (let attempt = 0; attempt < maxRetries; attempt++) {
350+
// Check if currentSearchResultId changed during retry loop (race condition)
351+
if (currentSearchResultId !== capturedTargetId) {
352+
return; // Exit early, a newer navigation is in progress
353+
}
354+
347355
// Check if we need to expand chunks
348-
const messageIndex = currentIndexMap.get(targetId);
356+
const messageIndex = currentIndexMap.get(capturedTargetId);
349357
350358
if (messageIndex !== undefined) {
351359
const itemsFromEnd = currentFlatItemsLength - messageIndex;
352360
const chunksNeeded = Math.ceil(itemsFromEnd / CHUNK_SIZE);
353361
354362
if (chunksNeeded > loadedChunksFromEnd) {
355363
loadedChunksFromEnd = chunksNeeded + 1;
364+
// Check again before async operation
365+
if (currentSearchResultId !== capturedTargetId) {
366+
return;
367+
}
356368
await tick(); // Wait for DOM update after chunk expansion
357369
}
358370
}
359371
360372
// Check if message ref exists in DOM
361-
if (messageRefs.has(targetId)) {
362-
const element = messageRefs.get(targetId);
373+
if (messageRefs.has(capturedTargetId)) {
374+
const element = messageRefs.get(capturedTargetId);
363375
364376
if (element) {
365377
highlightReady = false;
366378
highlightedId = null;
367-
pendingHighlightId = targetId;
379+
pendingHighlightId = capturedTargetId;
368380
isNavigationScroll = true;
369381
370382
element.scrollIntoView({ behavior: 'smooth', block: 'center' });
371383
384+
// Check again before async operation
385+
if (currentSearchResultId !== capturedTargetId) {
386+
return;
387+
}
388+
372389
setTimeout(() => {
373390
isNavigationScroll = false;
374391
}, 500);
@@ -377,7 +394,10 @@ $effect(() => {
377394
}
378395
}
379396
380-
// Ref not ready yet, wait and retry
397+
// Ref not ready yet, check again before waiting
398+
if (currentSearchResultId !== capturedTargetId) {
399+
return;
400+
}
381401
await new Promise((resolve) => setTimeout(resolve, retryDelay));
382402
}
383403
})();

src/lib/state.svelte.ts

Lines changed: 69 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
* Application state management using Svelte 5 runes
33
*/
44

5-
import type { ChatMessage } from './parser/chat-parser';
65
import type {
76
ParsedZipChat,
87
SerializedSearchMessage,
@@ -44,17 +43,18 @@ export function createAppState() {
4443
let searchResultIds = $state<string[]>([]); // Ordered list of matching message IDs (first N for navigation)
4544
let totalSearchMatches = $state(0); // Total number of matches (may be > searchResultIds.length)
4645
let currentSearchIndex = $state(0); // Current position in search results
47-
46+
4847
// Bitmap-based match lookup for O(1) performance
4948
// This avoids creating a Set from thousands of IDs on every search
5049
let searchMatchBitmap: Uint8Array | null = null;
5150
let searchMessageIdToIndex: Map<string, number> | null = null;
52-
51+
5352
let searchWorker: Worker | null = null;
5453
let searchWorkerReady = false;
5554
let searchWorkerChatTitle: string | null = null; // Track which chat the worker is loaded for
5655
let currentSearchId = 0; // For tracking/cancelling searches
5756
let searchDebounceId: ReturnType<typeof setTimeout> | null = null;
57+
let workerTimeoutId: ReturnType<typeof setTimeout> | null = null;
5858

5959
// Derived values
6060
const selectedChat = $derived(
@@ -91,6 +91,10 @@ export function createAppState() {
9191
// Terminate and cleanup worker
9292
function terminateSearchWorker() {
9393
cleanupSearchDebounce();
94+
if (workerTimeoutId) {
95+
clearTimeout(workerTimeoutId);
96+
workerTimeoutId = null;
97+
}
9498
if (searchWorker) {
9599
searchWorker.terminate();
96100
searchWorker = null;
@@ -101,7 +105,9 @@ export function createAppState() {
101105

102106
// Initialize or reuse search worker for a chat
103107
function ensureSearchWorker(chat: ChatData): Promise<void> {
104-
return new Promise((resolve) => {
108+
return new Promise((resolve, reject) => {
109+
let isSettled = false; // Track if promise is already settled
110+
105111
// If worker is already loaded for this chat, reuse it
106112
if (
107113
searchWorker &&
@@ -120,6 +126,26 @@ export function createAppState() {
120126
searchWorkerReady = false;
121127
searchWorkerChatTitle = chat.title;
122128

129+
// Set timeout to reject if worker doesn't respond within 5 seconds
130+
workerTimeoutId = setTimeout(() => {
131+
if (isSettled) return;
132+
isSettled = true;
133+
134+
// Terminate worker since initialization failed
135+
if (searchWorker) {
136+
searchWorker.terminate();
137+
searchWorker = null;
138+
}
139+
140+
searchWorkerReady = false;
141+
searchWorkerChatTitle = null;
142+
workerTimeoutId = null;
143+
144+
reject(
145+
new Error('Search worker failed to initialize within 5 seconds'),
146+
);
147+
}, 5000);
148+
123149
searchWorker = new Worker(
124150
new URL('./workers/search-worker.ts', import.meta.url),
125151
{ type: 'module' },
@@ -139,6 +165,13 @@ export function createAppState() {
139165
const data = event.data;
140166

141167
if (data.type === 'ready') {
168+
if (isSettled) return;
169+
isSettled = true;
170+
171+
if (workerTimeoutId) {
172+
clearTimeout(workerTimeoutId);
173+
workerTimeoutId = null;
174+
}
142175
searchWorkerReady = true;
143176
resolve();
144177
return;
@@ -164,7 +197,7 @@ export function createAppState() {
164197
} else {
165198
searchMatchBitmap = null;
166199
}
167-
200+
168201
searchResultIds = data.matchingIds ?? [];
169202
totalSearchMatches = data.totalMatches ?? searchResultIds.length;
170203
// Only update activeSearchQuery when results are ready
@@ -178,40 +211,46 @@ export function createAppState() {
178211

179212
searchWorker.onerror = (err) => {
180213
console.error('Search worker error:', err);
214+
if (isSettled) return;
215+
isSettled = true;
216+
217+
if (workerTimeoutId) {
218+
clearTimeout(workerTimeoutId);
219+
workerTimeoutId = null;
220+
}
181221
isSearching = false;
182222
searchProgress = 0;
183223
searchWorkerReady = false;
224+
reject(err);
184225
};
185226

186227
// Send simplified message data to worker
187228
// Only send id, content, sender - minimal data for search
188229
// This is much smaller than serializing the full MiniSearch index!
189-
setTimeout(() => {
190-
// Use pre-computed serializedMessages if available (faster, avoids reactive proxy)
191-
// Otherwise fall back to messages array
192-
const messageData = chat.serializedMessages
193-
? chat.serializedMessages.map((m) => ({
194-
id: m.id,
195-
content: m.content,
196-
sender: m.sender,
197-
}))
198-
: chat.messages.map((m) => ({
199-
id: m.id,
200-
content: m.content,
201-
sender: m.sender,
202-
}));
203-
204-
// Build the ID-to-index map for bitmap lookup on main thread
205-
searchMessageIdToIndex = new Map();
206-
for (let i = 0; i < messageData.length; i++) {
207-
searchMessageIdToIndex.set(messageData[i].id, i);
208-
}
230+
// Use pre-computed serializedMessages if available (faster, avoids reactive proxy)
231+
// Otherwise fall back to messages array
232+
const messageData = chat.serializedMessages
233+
? chat.serializedMessages.map((m) => ({
234+
id: m.id,
235+
content: m.content,
236+
sender: m.sender,
237+
}))
238+
: chat.messages.map((m) => ({
239+
id: m.id,
240+
content: m.content,
241+
sender: m.sender,
242+
}));
243+
244+
// Build the ID-to-index map for bitmap lookup on main thread
245+
searchMessageIdToIndex = new Map();
246+
for (let i = 0; i < messageData.length; i++) {
247+
searchMessageIdToIndex.set(messageData[i].id, i);
248+
}
209249

210-
searchWorker?.postMessage({
211-
type: 'load-data',
212-
messages: messageData,
213-
});
214-
}, 0);
250+
searchWorker.postMessage({
251+
type: 'load-data',
252+
messages: messageData,
253+
});
215254
});
216255
}
217256

src/lib/workers/index-worker.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* 3. Pre-serialized messages for search worker
88
*
99
* All operations are performed in a single worker pass for efficiency.
10-
*
10+
*
1111
* NOTE: We no longer use MiniSearch here. The search worker uses simple
1212
* string.includes() which is fast enough and avoids the overhead of
1313
* serializing/deserializing large index structures via postMessage.

src/routes/+page.svelte

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -211,19 +211,12 @@ async function handleFilesSelected(files: FileList) {
211211
}>;
212212
}>,
213213
) => {
214-
const {
215-
chatTitle,
216-
indexEntries,
217-
flatItems,
218-
serializedMessages,
219-
} = event.data;
214+
const { chatTitle, indexEntries, flatItems, serializedMessages } =
215+
event.data;
220216
const messageIndex = new Map(indexEntries);
221217
appState.updateChatMessageIndex(chatTitle, messageIndex);
222218
appState.updateChatFlatItems(chatTitle, flatItems);
223-
appState.updateChatSerializedMessages(
224-
chatTitle,
225-
serializedMessages,
226-
);
219+
appState.updateChatSerializedMessages(chatTitle, serializedMessages);
227220
indexWorker.terminate();
228221
};
229222

0 commit comments

Comments
 (0)