From 3220da556d0f20b85836395a6cde66a95d66cce2 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Tue, 25 Jun 2024 12:56:49 +0530 Subject: [PATCH] Use --- desktop/src/main/services/upload.ts | 9 +++--- desktop/src/main/services/zip.ts | 46 +++++++++++++++++++++++++++-- desktop/src/main/stream.ts | 6 ++-- desktop/src/main/utils/temp.ts | 6 ++-- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/desktop/src/main/services/upload.ts b/desktop/src/main/services/upload.ts index 3fffc68b5a..ecd50df779 100644 --- a/desktop/src/main/services/upload.ts +++ b/desktop/src/main/services/upload.ts @@ -1,14 +1,13 @@ -import StreamZip from "node-stream-zip"; import fs from "node:fs/promises"; import path from "node:path"; import { existsSync } from "original-fs"; import type { PendingUploads, ZipItem } from "../../types/ipc"; import log from "../log"; import { uploadStatusStore } from "../stores/upload-status"; -import { clearOpenZipCache } from "./zip"; +import { clearOpenZipCache, markClosableZip, openZip } from "./zip"; export const listZipItems = async (zipPath: string): Promise => { - const zip = new StreamZip.async({ file: zipPath }); + const zip = openZip(zipPath); const entries = await zip.entries(); const entryNames: string[] = []; @@ -22,7 +21,7 @@ export const listZipItems = async (zipPath: string): Promise => { } } - await zip.close(); + markClosableZip(zipPath); return entryNames.map((entryName) => [zipPath, entryName]); }; @@ -35,7 +34,7 @@ export const pathOrZipItemSize = async ( return stat.size; } else { const [zipPath, entryName] = pathOrZipItem; - const zip = new StreamZip.async({ file: zipPath }); + const zip = openZip(zipPath); const entry = await zip.entry(entryName); if (!entry) throw new Error( diff --git a/desktop/src/main/services/zip.ts b/desktop/src/main/services/zip.ts index 8cab702392..c8809af349 100644 --- a/desktop/src/main/services/zip.ts +++ b/desktop/src/main/services/zip.ts @@ -1,7 +1,27 @@ import { LRUCache } from "lru-cache"; import StreamZip from "node-stream-zip"; -const _cache = new LRUCache({ max: 50 }); +/** The cache. */ +const _cache = new LRUCache({ + max: 50, + disposeAfter: (zip, zipPath) => { + if (isInUse(zipPath)) { + // Add it back again. The `noDisposeOnSet` flag prevents dispose + // from being called again. From my understanding, it shouldn't + // matter in our case, but I've kept it here to match the + // recommended example: + // https://github.com/isaacs/node-lru-cache/issues/151#issuecomment-1033206697 + _cache.set(zipPath, zip); + } else { + void zip.close(); + } + }, +}); + +/** Reference count. */ +const _refCount = new Map(); + +const isInUse = (zipPath: string) => (_refCount.get(zipPath) ?? 0) > 0; /** * Cached `StreamZip.async`s @@ -29,10 +49,32 @@ export const openZip = (zipPath: string) => { result = new StreamZip.async({ file: zipPath }); _cache.set(zipPath, result); } + _refCount.set(zipPath, (_refCount.get(zipPath) ?? 0) + 1); return result; }; +/** + * Indicate to our cache that an item we opened earlier using {@link openZip} + * can now be safely closed. + * + * @param zipPath The key that was used for opening this zip. + */ +export const markClosableZip = (zipPath: string) => { + const rc = _refCount.get(zipPath); + if (!rc) throw new Error(`Double close for ${zipPath}`); + if (rc == 1) _refCount.delete(zipPath); + else _refCount.set(zipPath, rc - 1); +}; + /** * Clear any entries previously cached by {@link openZip}. */ -export const clearOpenZipCache = () => _cache.clear(); +export const clearOpenZipCache = () => { + if (_refCount.size > 0) { + const keys = JSON.stringify([..._refCount.keys()]); + throw new Error( + `Attempting to clear zip file cache when some items are still in use: ${keys}`, + ); + } + _cache.clear(); +}; diff --git a/desktop/src/main/stream.ts b/desktop/src/main/stream.ts index c11fb1121c..38feb192af 100644 --- a/desktop/src/main/stream.ts +++ b/desktop/src/main/stream.ts @@ -2,7 +2,6 @@ * @file stream data to-from renderer using a custom protocol handler. */ import { net, protocol } from "electron/main"; -import StreamZip from "node-stream-zip"; import { randomUUID } from "node:crypto"; import { createWriteStream, existsSync } from "node:fs"; import fs from "node:fs/promises"; @@ -11,6 +10,7 @@ import { ReadableStream } from "node:stream/web"; import { pathToFileURL } from "node:url"; import log from "./log"; import { ffmpegConvertToMP4 } from "./services/ffmpeg"; +import { markClosableZip, openZip } from "./services/zip"; import { ensure } from "./utils/common"; import { deleteTempFile, @@ -113,7 +113,7 @@ const handleRead = async (path: string) => { }; const handleReadZip = async (zipPath: string, entryName: string) => { - const zip = new StreamZip.async({ file: zipPath }); + const zip = openZip(zipPath); const entry = await zip.entry(entryName); if (!entry) return new Response("", { status: 404 }); @@ -130,7 +130,7 @@ const handleReadZip = async (zipPath: string, entryName: string) => { webReadableStreamAny as ReadableStream; // Close the zip handle when the underlying stream closes. - stream.on("end", () => void zip.close()); + stream.on("end", () => markClosableZip(zipPath)); // While it is documented that entry.time is the modification time, // the units are not mentioned. By seeing the source code, we can diff --git a/desktop/src/main/utils/temp.ts b/desktop/src/main/utils/temp.ts index 70dec844d6..912264387a 100644 --- a/desktop/src/main/utils/temp.ts +++ b/desktop/src/main/utils/temp.ts @@ -1,10 +1,10 @@ import { app } from "electron/main"; -import StreamZip from "node-stream-zip"; import { existsSync } from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; import type { ZipItem } from "../../types/ipc"; import log from "../log"; +import { markClosableZip, openZip } from "../services/zip"; import { ensure } from "./common"; /** @@ -128,9 +128,9 @@ export const makeFileForDataOrPathOrZipItem = async ( } else { writeToTemporaryFile = async () => { const [zipPath, entryName] = dataOrPathOrZipItem; - const zip = new StreamZip.async({ file: zipPath }); + const zip = openZip(zipPath); await zip.extract(entryName, path); - await zip.close(); + markClosableZip(zipPath); }; } }