Upgrade StorageConnector save-sessions are never garbage-collected (SessionManager.gc bug)
I think I've found a bug in the connector's save-session handling in chrome/content/zotero/xpcom/server/saveSession.js.
Zotero.Server.Connector.SessionManager.gc() is intended to expire old save-sessions, but it never frees any. The loop has two issues that compound:
1. It iterates the _sessions Map with for (let session of this._sessions). Map iteration yields [id, session] entry pairs, so session.created and session.id are always undefined and the age comparison never matches.
2. The delete in the body references this._session (singular) instead of this._sessions, so even if the condition matched it would throw rather than delete.
There's a related issue in SaveSession.remove(), which does delete Zotero.Server.Connector.SessionManager._sessions[this.id] — but _sessions is a Map, and the delete operator is a no-op on Maps, so explicit removal also fails.
Net effect: connector save-sessions accumulate in memory for the lifetime of the Zotero process rather than being expired after the intended TTL.
While looking at the surrounding server code I also noticed a likely-unrelated typo in server.js _processEndpoint's error handler — a misplaced parenthesis passes the content type and body to _requestFinished() instead of _generateResponse(), so the 500 error response is sent without its body.
I have a fix ready and can open a pull request against zotero/zotero if that's helpful — it's a four-line change across the two files. Happy to adjust the approach (or split out the server.js change) based on what you'd prefer.
Environment: current main (commit 06e16c2).
Zotero.Server.Connector.SessionManager.gc() is intended to expire old save-sessions, but it never frees any. The loop has two issues that compound:
1. It iterates the _sessions Map with for (let session of this._sessions). Map iteration yields [id, session] entry pairs, so session.created and session.id are always undefined and the age comparison never matches.
2. The delete in the body references this._session (singular) instead of this._sessions, so even if the condition matched it would throw rather than delete.
There's a related issue in SaveSession.remove(), which does delete Zotero.Server.Connector.SessionManager._sessions[this.id] — but _sessions is a Map, and the delete operator is a no-op on Maps, so explicit removal also fails.
Net effect: connector save-sessions accumulate in memory for the lifetime of the Zotero process rather than being expired after the intended TTL.
While looking at the surrounding server code I also noticed a likely-unrelated typo in server.js _processEndpoint's error handler — a misplaced parenthesis passes the content type and body to _requestFinished() instead of _generateResponse(), so the 500 error response is sent without its body.
I have a fix ready and can open a pull request against zotero/zotero if that's helpful — it's a four-line change across the two files. Happy to adjust the approach (or split out the server.js change) based on what you'd prefer.
Environment: current main (commit 06e16c2).
-
dstillman Zotero TeamBetter to post development questions to zotero-dev, but if you're confident you've identified a bug (and this isn't just some unverified AI analysis), you can open a PR.