From b918ad2892d2853f0511a26aa0cace2bd6e5ea4f Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 31 Aug 2023 06:04:46 -0400
Subject: [PATCH] Fix error closing ZIP reader during file sync on Windows

In Z7 on Windows 10 (but not 11 for me), nsIZipReader doesn't properly
close the file after `findEntries()` is called (as discovered by
@abaevbog), so a `remove()` on the downloaded ZIP file during file
syncing triggers an access-denied error. Setting the zip-reader variable
to null and forcing garbage collection seems to fix it. Doing this
everywhere we use nsIZipReader just to be safe.

I found the `forceGC()` in only one test file in fx102, but setting the
reader to null is done more widely, so maybe they just don't try to
delete ZIP files before GC happens and manage to avoid this bug.

Fixes #3369
---
 chrome/content/zotero/EPUB.jsm                     |  2 ++
 chrome/content/zotero/xpcom/dictionaries.js        |  2 ++
 .../content/zotero/xpcom/storage/storageLocal.js   | 14 ++++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/chrome/content/zotero/EPUB.jsm b/chrome/content/zotero/EPUB.jsm
index b0b4e731a7..22f154e23a 100644
--- a/chrome/content/zotero/EPUB.jsm
+++ b/chrome/content/zotero/EPUB.jsm
@@ -56,6 +56,8 @@ class EPUB {
 
 	close() {
 		this._zipReader.close();
+		this._zipReader = null
+		Cu.forceGC();
 	}
 
 	async* getSectionDocuments() {
diff --git a/chrome/content/zotero/xpcom/dictionaries.js b/chrome/content/zotero/xpcom/dictionaries.js
index 74df1b3a89..4cdabf0cb0 100644
--- a/chrome/content/zotero/xpcom/dictionaries.js
+++ b/chrome/content/zotero/xpcom/dictionaries.js
@@ -136,6 +136,8 @@ Zotero.Dictionaries = new function () {
 			}
 
 			zipReader.close();
+			zipReader = null
+			Cu.forceGC();
 			await OS.File.remove(xpiPath);
 			await _loadDirectory(dir);
 		}
diff --git a/chrome/content/zotero/xpcom/storage/storageLocal.js b/chrome/content/zotero/xpcom/storage/storageLocal.js
index 2aef651d84..7787deadc4 100644
--- a/chrome/content/zotero/xpcom/storage/storageLocal.js
+++ b/chrome/content/zotero/xpcom/storage/storageLocal.js
@@ -794,6 +794,8 @@ Zotero.Sync.Storage.Local = {
 		catch (e) {
 			Zotero.debug(zipFile.leafName + " is not a valid ZIP file", 2);
 			zipReader.close();
+			zipReader = null
+			Cu.forceGC();
 			
 			try {
 				zipFile.remove(false);
@@ -813,6 +815,8 @@ Zotero.Sync.Storage.Local = {
 		}
 		catch (e) {
 			zipReader.close();
+			zipReader = null
+			Cu.forceGC();
 			throw e;
 		}
 		
@@ -900,6 +904,8 @@ Zotero.Sync.Storage.Local = {
 				Zotero.logError(e);
 				
 				zipReader.close();
+				zipReader = null
+				Cu.forceGC();
 				
 				Zotero.File.checkFileAccessError(e, destPath, 'create');
 			}
@@ -914,6 +920,9 @@ Zotero.Sync.Storage.Local = {
 					}
 					catch (e) {}
 					zipReader.close();
+					zipReader = null
+					Cu.forceGC();
+					
 					// TODO: localize
 					var msg = "Due to a Windows path length limitation, your Zotero data directory "
 						+ "is too deep in the filesystem for syncing to work reliably. "
@@ -950,6 +959,8 @@ Zotero.Sync.Storage.Local = {
 				}
 				
 				zipReader.close();
+				zipReader = null
+				Cu.forceGC();
 				
 				Zotero.File.checkFileAccessError(e, destPath, 'create');
 			}
@@ -962,6 +973,9 @@ Zotero.Sync.Storage.Local = {
 			}
 		}
 		zipReader.close();
+		zipReader = null
+		Cu.forceGC();
+		
 		zipFile.remove(false);
 		
 		return returnFile;