From 5269c11ee08a671680943c03b7055dab1a6f4c87 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Tue, 4 Apr 2017 23:22:13 -0700 Subject: [PATCH] [client-sync] Prevent IMAP connection leaking in sync worker Summary: When the sync worker got stuck inside `performSync`, we would never release the connection back to the connection pool. We would then try to start a new sync worker, and it would claim a new connection. If this happened enough times, we would eventually exhaust the whole pool and keep the sync worker blocked forever. Test Plan: manual Reviewers: evan, spang, halla, mark Reviewed By: halla, mark Differential Revision: https://phab.nylas.com/D4359 --- .../src/local-sync-worker/sync-worker.es6 | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/client-sync/src/local-sync-worker/sync-worker.es6 b/packages/client-sync/src/local-sync-worker/sync-worker.es6 index 61181c9f9..42c460f81 100644 --- a/packages/client-sync/src/local-sync-worker/sync-worker.es6 +++ b/packages/client-sync/src/local-sync-worker/sync-worker.es6 @@ -32,10 +32,10 @@ class SyncWorker { constructor(account, db, syncProcessManager) { this._db = db; this._manager = syncProcessManager; - this._mainIMAPConn = null; this._smtp = null; this._account = account; this._currentTask = null + this._mainIMAPConn = null; this._mailListenerIMAPConn = null this._interruptible = new Interruptible() this._logger = global.Logger.forAccount(account) @@ -51,7 +51,8 @@ class SyncWorker { this._requireTokenRefresh = false this._batchProcessedUids = new Map(); this._latestOpenTimesByFolder = new Map(); - this._mailListenerIMAPConnDisposeFn = null + this._mainIMAPConnDisposer = null + this._mailListenerIMAPConnDisposer = null this._retryScheduler = new ExponentialBackoffScheduler({ baseDelay: 15 * 1000, @@ -275,7 +276,7 @@ class SyncWorker { this._onInboxUpdates(`There are flag updates on the inbox`); }) - this._mailListenerIMAPConnDisposeFn = done + this._mailListenerIMAPConnDisposer = done // Return true to keep connection open return true }, @@ -304,13 +305,17 @@ class SyncWorker { _disposeMainIMAPConnection() { this._mainIMAPConn = null; + if (this._mainIMAPConnDisposer) { + this._mainIMAPConnDisposer() + this._mainIMAPConnDisposer = null + } } _disposeMailListenerIMAPConnection() { this._mailListenerIMAPConn = null; - if (this._mailListenerIMAPConnDisposeFn) { - this._mailListenerIMAPConnDisposeFn() - this._mailListenerIMAPConnDisposeFn = null + if (this._mailListenerIMAPConnDisposer) { + this._mailListenerIMAPConnDisposer() + this._mailListenerIMAPConnDisposer = null } } @@ -593,9 +598,11 @@ class SyncWorker { desiredCount: 1, logger: this._logger, socketTimeout: this._retryScheduler.currentDelay(), - onConnected: async ([mainConn]) => { + onConnected: async ([mainConn], done) => { + this._mainIMAPConnDisposer = done await this._ensureMainIMAPConnection(mainConn); await this._interruptible.run(this._performSync, this) + this._mainIMAPConnDisposer = null }, }); @@ -609,7 +616,7 @@ class SyncWorker { } finally { this._lastSyncTime = Date.now() this._syncInProgress = false - this._mainIMAPConn = null; + this._disposeMainIMAPConnection({errored: false}) await this._scheduleNextSync(error) } }