From b0f690ba5585374ed945c23e47149f3ce7f6d3ad Mon Sep 17 00:00:00 2001 From: cjihrig Date: Tue, 3 Dec 2019 23:54:35 -0500 Subject: [PATCH 1/3] fs: add synchronous retries to rimraf This commit gives the synchronous version of rimraf the same linear retry logic as the asynchronous version. Prior to this commit, sync rimraf kept retrying the operation as soon as possible until maxRetries was reached. --- lib/internal/fs/rimraf.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/internal/fs/rimraf.js b/lib/internal/fs/rimraf.js index 60310e1cf9427e..350859d6b5989f 100644 --- a/lib/internal/fs/rimraf.js +++ b/lib/internal/fs/rimraf.js @@ -21,6 +21,7 @@ const { } = require('fs'); const { join } = require('path'); const { setTimeout } = require('timers'); +const { sleep } = require('internal/util'); const notEmptyErrorCodes = new Set(['ENOTEMPTY', 'EEXIST', 'EPERM']); const retryErrorCodes = new Set( ['EBUSY', 'EMFILE', 'ENFILE', 'ENOTEMPTY', 'EPERM']); @@ -208,10 +209,12 @@ function _rmdirSync(path, options, originalErr) { rimrafSync(join(path, child), options); }); - for (let i = 0; i < options.maxRetries + 1; i++) { + for (let i = 1; i <= options.maxRetries + 1; i++) { try { return rmdirSync(path, options); - } catch {} // Ignore errors. + } catch { + sleep(i * options.retryDelay); + } } } } From 8c2ca28a8d449a79c1e0214c675e71df9fac17f9 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 8 Dec 2019 09:54:01 -0500 Subject: [PATCH 2/3] fixup! fs: add synchronous retries to rimraf --- lib/internal/fs/rimraf.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/fs/rimraf.js b/lib/internal/fs/rimraf.js index 350859d6b5989f..d6330fbe4399a0 100644 --- a/lib/internal/fs/rimraf.js +++ b/lib/internal/fs/rimraf.js @@ -213,7 +213,8 @@ function _rmdirSync(path, options, originalErr) { try { return rmdirSync(path, options); } catch { - sleep(i * options.retryDelay); + if (options.retryDelay > 0) + sleep(i * options.retryDelay); } } } From 2dd26b08d1c0dc99626f92571259475b87a2029a Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 8 Dec 2019 10:50:04 -0500 Subject: [PATCH 3/3] fs: reduce unnecessary sync rimraf retries rimraf should only retry if certain errors are encountered. Additionally, there is no point sleeping if an error occurs on the last try. --- lib/internal/fs/rimraf.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/internal/fs/rimraf.js b/lib/internal/fs/rimraf.js index d6330fbe4399a0..6d6b19efcc98f2 100644 --- a/lib/internal/fs/rimraf.js +++ b/lib/internal/fs/rimraf.js @@ -209,12 +209,19 @@ function _rmdirSync(path, options, originalErr) { rimrafSync(join(path, child), options); }); - for (let i = 1; i <= options.maxRetries + 1; i++) { + const tries = options.maxRetries + 1; + + for (let i = 1; i <= tries; i++) { try { return rmdirSync(path, options); - } catch { - if (options.retryDelay > 0) + } catch (err) { + // Only sleep if this is not the last try, and the delay is greater + // than zero, and an error was encountered that warrants a retry. + if (retryErrorCodes.has(err.code) && + i < tries && + options.retryDelay > 0) { sleep(i * options.retryDelay); + } } } }