Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions patches/@jsr__meshtastic__core@2.6.6.patch
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,74 @@ index ddca80fc9134fa56ac273b17387e62af087c95c6..365b64b0e60cce8f23cb9d6a6d463b6d
await this.transport.disconnect();
}

diff --git a/src/utils/queue.js b/src/utils/queue.js
index d6082a6401e15eeb3e8873820b64341c961ac35f..73457e9e6fbde50800af427bb82b53b59049a7df 100755
--- a/src/utils/queue.js
+++ b/src/utils/queue.js
@@ -55,10 +55,8 @@ export class Queue {
this.queue.push(queueItem);
}
remove(id) {
- if (this.lock) {
- setTimeout(()=>this.remove(id), 100);
- return;
- }
+ // mesh-client patch: see src/utils/queue.ts for rationale — this used to deadlock
+ // whenever processQueue never released `lock`.
this.queue = this.queue.filter((item)=>item.id !== id);
}
processAck(id) {
@@ -88,10 +86,12 @@ export class Queue {
await new Promise((resolve)=>setTimeout(resolve, 200));
try {
await writer.write(item.data);
- item.sent = true;
} catch (error) {
console.error(`Error sending packet ${item.id}`, error);
}
+ // mesh-client patch: see src/utils/queue.ts for rationale — mark attempted either
+ // way so one write failure can't wedge every other queued packet behind it.
+ item.sent = true;
}
}
} finally{
diff --git a/src/utils/queue.ts b/src/utils/queue.ts
index 9c0d192d0a22c1a158e70f4dc3b74460ea7bd22b..7d63cb22ad5c6a8cde8823b9925e93b63f8cdeb9 100755
--- a/src/utils/queue.ts
+++ b/src/utils/queue.ts
@@ -70,10 +70,11 @@ export class Queue {
}

public remove(id: number): void {
- if (this.lock) {
- setTimeout(() => this.remove(id), 100);
- return;
- }
+ // mesh-client patch: `queue` mutation is synchronous and single-threaded, so there is
+ // no real race to guard against here. Gating on `lock` deadlocked whenever
+ // `processQueue` never released it (see the processQueue patch below): the item's own
+ // 60s ack/timeout in push() could never actually remove it, so processQueue kept
+ // re-picking the same still-queued item forever.
this.queue = this.queue.filter((item) => item.id !== id);
}

@@ -115,10 +116,18 @@ export class Queue {
await new Promise((resolve) => setTimeout(resolve, 200));
try {
await writer.write(item.data);
- item.sent = true;
} catch (error) {
console.error(`Error sending packet ${item.id}`, error);
}
+ // mesh-client patch: mark the item attempted whether or not the write succeeded.
+ // The original code only did this on success, so a single transport-level write
+ // failure (e.g. socket briefly gone during a reconnect) left the item permanently
+ // unsent — this loop kept re-picking the same first-in-line item forever at the
+ // 200ms cadence above, holding `lock` and starving every other queued packet
+ // (including heartbeats) behind it. The item's own promise still settles normally
+ // via the ack/60s-timeout machinery in push(); this only stops it from wedging
+ // every *other* packet.
+ item.sent = true;
}
}
} finally {
10 changes: 5 additions & 5 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 35 additions & 0 deletions src/main/index.ipc-security.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,25 @@ describe('meshtastic:tcp-write byte validation (source contract)', () => {
const handlerBody = INDEX_SOURCE.slice(handlerIdx, handlerIdx + 1200);
expect(handlerBody).toContain('meshtasticTcpSocket.destroy()');
});

it('emits meshtastic:tcp-disconnected only for the active socket (PR #792)', () => {
// connect/disconnect null the ref before destroy(); a superseded close must not broadcast
// or the renderer TCP loss-watch will tear down a healthy replacement session.
const handlerIdx = INDEX_SOURCE.indexOf("ipcMain.handle('meshtastic:tcp-connect'");
expect(handlerIdx).toBeGreaterThan(-1);
const closeIdx = INDEX_SOURCE.indexOf("socket.on('close'", handlerIdx);
expect(closeIdx).toBeGreaterThan(handlerIdx);
const closeBody = INDEX_SOURCE.slice(closeIdx, closeIdx + 600);
expect(closeBody).toContain('if (meshtasticTcpSocket === socket)');
expect(closeBody).toContain("mainWindow?.webContents.send('meshtastic:tcp-disconnected')");
// Emit must be inside the active-socket guard (not before it).
const guardIdx = closeBody.indexOf('if (meshtasticTcpSocket === socket)');
const emitIdx = closeBody.indexOf(
"mainWindow?.webContents.send('meshtastic:tcp-disconnected')",
);
expect(guardIdx).toBeGreaterThan(-1);
expect(emitIdx).toBeGreaterThan(guardIdx);
});
});

// ─── meshcore:tcp-write byte element validation ──────────────────────
Expand Down Expand Up @@ -323,6 +342,22 @@ describe('meshcore:tcp-connect hostname validation (source contract)', () => {
const handlerBody = INDEX_SOURCE.slice(handlerIdx, handlerIdx + 800);
expect(handlerBody).toContain('formatHostForSocket(');
});

it('emits meshcore:tcp-disconnected only for the active socket (PR #792)', () => {
// Same contract as meshtastic:tcp-connect — superseded closes from connect-replace /
// disconnect must not look like a live link drop to the renderer reconnect path.
const handlerIdx = INDEX_SOURCE.indexOf("ipcMain.handle('meshcore:tcp-connect'");
expect(handlerIdx).toBeGreaterThan(-1);
const closeIdx = INDEX_SOURCE.indexOf("socket.on('close'", handlerIdx);
expect(closeIdx).toBeGreaterThan(handlerIdx);
const closeBody = INDEX_SOURCE.slice(closeIdx, closeIdx + 600);
expect(closeBody).toContain('if (meshcoreTcpSocket === socket)');
expect(closeBody).toContain("mainWindow?.webContents.send('meshcore:tcp-disconnected')");
const guardIdx = closeBody.indexOf('if (meshcoreTcpSocket === socket)');
const emitIdx = closeBody.indexOf("mainWindow?.webContents.send('meshcore:tcp-disconnected')");
expect(guardIdx).toBeGreaterThan(-1);
expect(emitIdx).toBeGreaterThan(guardIdx);
});
});

// ─── meshtastic:tcp-connect hostname validation ──────────────────────
Expand Down
18 changes: 14 additions & 4 deletions src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6123,8 +6123,13 @@ ipcMain.handle('meshcore:tcp-connect', (event, host: string, port: number) => {
socket.on('close', (hadError) => {
clearTimeout(connectTimeout);
console.debug('[IPC] meshcore:tcp socket closed', hadError ? '(hadError)' : '(clean)');
mainWindow?.webContents.send('meshcore:tcp-disconnected');
if (meshcoreTcpSocket === socket) meshcoreTcpSocket = null;
// Only notify when this socket is still the active bridge. connect/disconnect clear the
// ref before destroy(), so superseded closes must not look like a live link drop
// (renderer reconnect is driven by this event — see #792).
if (meshcoreTcpSocket === socket) {
meshcoreTcpSocket = null;
mainWindow?.webContents.send('meshcore:tcp-disconnected');
}
});
socket.on('error', (err) => {
clearTimeout(connectTimeout);
Expand Down Expand Up @@ -6231,8 +6236,13 @@ ipcMain.handle('meshtastic:tcp-connect', (event, host: string, port: number) =>
socket.on('close', (hadError) => {
clearTimeout(connectTimeout);
console.debug('[IPC] meshtastic:tcp socket closed', hadError ? '(hadError)' : '(clean)');
mainWindow?.webContents.send('meshtastic:tcp-disconnected');
if (meshtasticTcpSocket === socket) meshtasticTcpSocket = null;
// Only notify when this socket is still the active bridge. connect/disconnect clear the
// ref before destroy(), so superseded closes must not look like a live link drop
// (renderer reconnect is driven by this event — see #792).
if (meshtasticTcpSocket === socket) {
meshtasticTcpSocket = null;
mainWindow?.webContents.send('meshtastic:tcp-disconnected');
}
});
socket.on('error', (err) => {
clearTimeout(connectTimeout);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,59 @@ describe('meshtasticTransportLossDetection', () => {
expect(onLost).toHaveBeenCalledTimes(1);
});

it('notifies immediately on main-process TCP socket disconnect', () => {
// Regression: writes that fail after the socket is gone report "no active socket",
// which does not match TRANSPORT_LOST_MESSAGE, so this IPC event is TCP's only fast
// path — without it, TCP relied solely on the passive watchdog (up to 3 minutes).
let capturedCb: (() => void) | undefined;
const spy = vi
.spyOn(window.electronAPI.meshtastic.tcp, 'onDisconnected')
.mockImplementation((cb) => {
capturedCb = cb;
return () => {};
});

const onLost = vi.fn();
const inner = new WritableStream<Uint8Array>({ write: vi.fn() });
const device = {
transport: { toDevice: inner },
} as unknown as MeshDevice;

try {
attachMeshtasticTransportLossWatch(device, 'tcp', onLost);
expect(spy).toHaveBeenCalledTimes(1);

capturedCb?.();

expect(onLost).toHaveBeenCalledTimes(1);
} finally {
spy.mockRestore();
}
});

it('unsubscribes the TCP disconnect listener on cleanup', () => {
const unsub = vi.fn();
const spy = vi
.spyOn(window.electronAPI.meshtastic.tcp, 'onDisconnected')
.mockReturnValue(unsub);

const inner = new WritableStream<Uint8Array>({ write: vi.fn() });
const device = {
transport: { toDevice: inner },
} as unknown as MeshDevice;

try {
const detach = attachMeshtasticTransportLossWatch(device, 'tcp', vi.fn());
expect(unsub).not.toHaveBeenCalled();

detach();

expect(unsub).toHaveBeenCalledTimes(1);
} finally {
spy.mockRestore();
}
});

it('serializes concurrent getWriter calls without WritableStream locked errors', async () => {
let innerWriteCount = 0;
const inner = new WritableStream<Uint8Array>({
Expand Down
12 changes: 12 additions & 0 deletions src/renderer/lib/meshtastic/meshtasticTransportLossDetection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,18 @@ export function attachMeshtasticTransportLossWatch(
}
}

if (type === 'tcp') {
// Main process reports the socket's own 'close'/'error' event within milliseconds of the
// real network failure (clean FIN or RST alike). Without this, TCP relied solely on the
// passive stale/dead watchdog noticing silence — up to 3 minutes after a connection that
// was already gone — because writes that fail with "no active socket" don't match the
// TRANSPORT_LOST_MESSAGE regex below, so this is the only fast path for TCP.
const unsubTcpDisconnected = window.electronAPI.meshtastic.tcp.onDisconnected(() => {
notify('tcp-socket-closed');
});
cleanups.push(unsubTcpDisconnected);
}

const transport = device.transport as { toDevice?: WritableStream<Uint8Array> } | undefined;
if (transport?.toDevice) {
const transportObj = device.transport as object;
Expand Down
6 changes: 4 additions & 2 deletions src/renderer/lib/timeConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,10 @@ export const MESHTASTIC_GET_METADATA_AFTER_CONFIGURE_RETRY_MS = 8_000;
export const MESHTASTIC_BLE_CONFIGURE_TIMEOUT_MS = 30 * MS_PER_SECOND;

/**
* Hard ceiling for one LoRa BLE reconnect open+handshake attempt (Meshtastic + MeshCore).
* Covers darwin dual createBleConnection attempts (~45–50s) + configure/attach margin so
* Hard ceiling for one LoRa reconnect open+configure/attach attempt (Meshtastic + MeshCore),
* applied to every transport (name is historical — BLE was the only transport with a deadline
* at all until TCP/serial/HTTP reconnects were found hanging indefinitely with none). For BLE,
* covers darwin dual createBleConnection attempts (~45–50s) + configure/attach margin so
* deferred Noble disconnect flush always runs instead of stalling retries at edge of range.
*/
export const NOBLE_BLE_RECONNECT_ATTEMPT_BUDGET_MS =
Expand Down
33 changes: 28 additions & 5 deletions src/renderer/runtime/useMeshcoreRuntime.reconnect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,28 @@ describe('useMeshcoreRuntime auto-reconnect (regression)', () => {
);
});

it('bounds BLE reconnect open+attach with NOBLE_BLE_RECONNECT_ATTEMPT_BUDGET_MS', () => {
it('bounds every reconnect open+attach with NOBLE_BLE_RECONNECT_ATTEMPT_BUDGET_MS', () => {
// Applies to all transports, not just BLE (see comment at the call site): TCP/serial used
// to await the open+attach attempt with no ceiling at all, so a hang anywhere in that
// sequence (e.g. a disconnect landing mid-attach) wedged reconnection forever — and unlike
// serial, MeshCore TCP has no fallback watchdog either (see meshcoreSerialWatchdog).
expect(RUNTIME_SOURCE).toContain('NOBLE_BLE_RECONNECT_ATTEMPT_BUDGET_MS');
expect(RUNTIME_SOURCE).toContain('raceWithDeadline');
const reconnectBody = extractUseCallbackBody(RUNTIME_SOURCE, 'attemptMeshcoreReconnect');
expect(reconnectBody).toContain('raceWithDeadline');
expect(reconnectBody).toContain('BLE reconnect attempt timed out after');
expect(reconnectBody).toContain('Reconnect attempt timed out after');
expect(reconnectBody).toContain('attemptActive');
expect(reconnectBody).toContain('meshcoreReconnectConnectInFlightRef.current = true');
expect(reconnectBody).not.toContain('if (isBleReconnect) {\n await raceWithDeadline');
});

it('on BLE reconnect timeout invalidates setup generation and cleans late transports', () => {
it('on reconnect timeout invalidates setup generation and cleans late transports (any transport, CodeRabbit #792)', () => {
// meshcoreSetupGenerationRef guards background initConn RPCs (getSelfInfo/getContacts/
// getChannels/etc.) generically — not BLE-specific (see its other call sites). Gating the
// bump on isBleReconnect here was only ever correct while raceWithDeadline itself was
// BLE-only; now that every transport's reconnect races the same deadline, a timed-out
// TCP/serial attempt must invalidate the setup generation too, or its background RPCs keep
// running and can apply stale state after the attempt was already declared failed.
expect(RUNTIME_SOURCE).toContain('createBleReconnectTransportCleanup');
const reconnectBody = extractUseCallbackBody(RUNTIME_SOURCE, 'attemptMeshcoreReconnect');
expect(reconnectBody).toMatch(
Expand All @@ -127,9 +138,12 @@ describe('useMeshcoreRuntime auto-reconnect (regression)', () => {
expect(reconnectBody).toMatch(
/lateTransport\.cleanup\(opened\.driverIdentityId\);\s*throw new Error\('MeshCore reconnect superseded during attach'\)/,
);
expect(reconnectBody).toMatch(
/catch \(err\) \{[\s\S]*?isBleReconnect[\s\S]*?meshcoreSetupGenerationRef\.current \+= 1/,
const catchBody = reconnectBody.slice(
reconnectBody.indexOf('} catch (err) {'),
reconnectBody.indexOf('await lateTransport.cleanup(opened?.driverIdentityId)'),
);
expect(catchBody).toContain('meshcoreSetupGenerationRef.current += 1');
expect(catchBody).not.toContain('if (isBleReconnect)');
});

it('cleans up transport when RF link is lost after reconnect attach', () => {
Expand Down Expand Up @@ -242,6 +256,15 @@ describe('useMeshcoreRuntime auto-reconnect (regression)', () => {
expect(RUNTIME_SOURCE).toContain('startSerialRediscovery');
expect(RUNTIME_SOURCE).toContain('captureSerialIdentityForRediscovery');
});

it('notifies immediately on main-process TCP socket disconnect (regression)', () => {
// Unlike serial, MeshCore's TCP transport has no fallback watchdog at all (see
// startMeshcoreSerialWatchdog, gated on rfType === 'serial'), so meshcore.tcp.onDisconnected
// is the only automatic recovery path for a dropped TCP connection.
expect(RUNTIME_SOURCE).toMatch(
/window\.electronAPI\.meshcore\.tcp\.onDisconnected\(\(\) => \{[\s\S]*?rfType !== 'tcp'[\s\S]{0,200}handleMeshcoreConnectionLostRef\.current\(\)/,
);
});
});

describe('useMeshcoreRuntime manual disconnect must not auto-reconnect', () => {
Expand Down
Loading