fix(gridstore-upload): ZMSA-17: fix gridstore upload deadlock (#918)

* fix gridstore upload deadlock

* releaseLock add callback if no calculatedFileContentHash

* gridstore-storage improve flow, refactor

* gridstore create handle case if lock variable is set but lock was not acquired

* gridstore attachmentCallback refactor

* refactor already returned logic in gristore create

Author: NickOvt

lib/attachments/gridstore-storage.js

@@ -154,36 +154,40 @@ class GridstoreStorage {
                 calculatedFileContentHash = args[2];
             }
 
+            const finalizeCallback = () => {
+                if (returned) {
+                    // might be already finished if retrying after delay
+                    return;
+                }
+
+                returned = true;
+                if (calculatedFileContentHash) {
+                    this.updateFileWithContentHashMetadata(args, hash, calculatedFileContentHash, callback);
+                    return;
+                }
+
+                callback(...args);
+            };
+
             if (storeLock) {
                 log.silly('GridStore', '[%s] UNLOCK lock=%s status=%s', instance, lockId, storeLock.success ? 'locked' : 'empty');
+
                 if (storeLock.success) {
-                    this.lock.releaseLock(storeLock, () => {
-                        if (returned) {
-                            // might be already finished if retrying after delay
-                            return;
-                        }
-                        if (calculatedFileContentHash) {
-                            // locked upload, new file
-                            this.updateFileWithContentHashMetadata(args, hash, calculatedFileContentHash, callback);
-                            return; // return from attachmentCallback. Top level callback will be ran after hash update
-                        }
-                    });
-                    // unset variable to prevent double releasing
-                    storeLock = false;
-                    return;
+                    // lock acquired
+                    this.lock.releaseLock(storeLock, finalizeCallback);
                 }
+
+                if (!storeLock.success) {
+                    // lock was not acquired
+                    finalizeCallback();
+                }
+
+                // unset variable to prevent double releasing
                 storeLock = false;
-            }
-            if (returned) {
-                // might be already finished if retrying after delay
                 return;
             }
-            if (calculatedFileContentHash) {
-                // no lock upload, new file
-                this.updateFileWithContentHashMetadata(args, hash, calculatedFileContentHash, callback);
-                return; // return from attachmentCallback. Top level callback will be ran after hash update
-            }
-            callback(...args);
+
+            finalizeCallback();
         };
 
         let tryCount = 0;
@@ -245,7 +249,6 @@ class GridstoreStorage {
                                 if (returned) {
                                     return;
                                 }
-                                returned = true;
                                 return attachmentCallback(err);
                             }
 
@@ -266,56 +269,56 @@ class GridstoreStorage {
                             if (returned) {
                                 return;
                             }
-                            if (err.code === 11000) {
-                                // most probably a race condition, try again
-                                if (tryCount++ < 5) {
-                                    if (/\.chunks /.test(err.message)) {
-                                        // Partial chunks detected. Might be because of:
-                                        // * another process is inserting the same attachment and thus no "files" entry yet (should not happened though due to locking)
-                                        // * previously deleted attachment that has not been properly removed
-
-                                        // Load data for an existing chunk to see the age of it
-                                        return this.gridfs.collection(this.bucketName + '.chunks').findOne(
-                                            {
-                                                files_id: hash
-                                            },
-                                            {
-                                                projection: {
-                                                    _id: true
-                                                }
-                                            },
-                                            (err, data) => {
-                                                if (err) {
-                                                    // whatever
-                                                    return setTimeout(tryStore, 100 + 200 * Math.random());
-                                                }
-
-                                                if (!data || !data._id) {
-                                                    // try again, no chunks found
-                                                    return setTimeout(tryStore, 10);
-                                                }
+                            // most probably a race condition, try again
+                            if (tryCount++ < 5) {
+                                if (/\.chunks /.test(err.message)) {
+                                    // Partial chunks detected. Might be because of:
+                                    // * another process is inserting the same attachment and thus no "files" entry yet (should not happened though due to locking)
+                                    // * previously deleted attachment that has not been properly removed
+
+                                    // Load data for an existing chunk to see the age of it
+                                    return this.gridfs.collection(this.bucketName + '.chunks').findOne(
+                                        {
+                                            files_id: hash
+                                        },
+                                        {
+                                            projection: {
+                                                _id: true
+                                            }
+                                        },
+                                        (err, data) => {
+                                            if (err) {
+                                                // whatever
+                                                return setTimeout(tryStore, 100 + 200 * Math.random());
+                                            }
 
-                                                // Check how old is the previous chunk
-                                                let timestamp = data._id.getTimestamp();
-                                                if (timestamp && typeof timestamp.getTime === 'function' && timestamp.getTime() >= Date.now() - 5 * 60 * 1000) {
-                                                    // chunk is newer than 5 minutes, assume race condition and try again after a while
-                                                    return setTimeout(tryStore, 300 + 200 * Math.random());
-                                                }
+                                            if (!data || !data._id) {
+                                                // try again, no chunks found
+                                                return setTimeout(tryStore, 10);
+                                            }
 
-                                                // partial chunks for a probably deleted message detected, try to clean up
-                                                setTimeout(() => {
-                                                    if (returned) {
-                                                        return;
-                                                    }
-                                                    this.cleanupGarbage(id, tryStore);
-                                                }, 100 + 200 * Math.random());
+                                            // Check how old is the previous chunk
+                                            let timestamp = data._id.getTimestamp();
+                                            if (timestamp && typeof timestamp.getTime === 'function' && timestamp.getTime() >= Date.now() - 5 * 60 * 1000) {
+                                                // chunk is newer than 5 minutes, assume race condition and try again after a while
+                                                return setTimeout(tryStore, 300 + 200 * Math.random());
                                             }
-                                        );
-                                    }
+
+                                            // partial chunks for a probably deleted message detected, try to clean up
+                                            setTimeout(() => {
+                                                if (returned) {
+                                                    return;
+                                                }
+                                                this.cleanupGarbage(id, tryStore);
+                                            }, 100 + 200 * Math.random());
+                                        }
+                                    );
+                                } else {
                                     return setTimeout(tryStore, 10);
                                 }
+                            } else {
+                                attachmentCallback(err);
                             }
-                            attachmentCallback(err);
                         });
 
                         store.once('finish', () => attachmentCallback(null, id, fileHashCalculator.hash));