We received a report that an update of Tor Browser failed with the message: "ensure_copy_recursive: path is not a directory: .../Cache/4/99, rv: 0, err: 24" Errno 24 is EMFILE aka "Too many open files." In reviewing the code, we found that ensure_copy_recursive() in updater.cpp is missing a call to closedir(). Something like this should fix the problem: --- a/toolkit/mozapps/update/updater/updater.cpp +++ b/toolkit/mozapps/update/updater/updater.cpp @@ -797,16 +797,17 @@ static int ensure_copy_recursive(const NS_tchar *path, const NS_tchar *dest, NS_T("%s/%s"), dest, entry->d_name); rv = ensure_copy_recursive(childPath, childPathDest, skiplist); if (rv) { break; } } } + NS_tclosedir(dir); return rv; } // Renames the specified file to the new file specified. If the destination file // exists it is removed. static int rename_file(const NS_tchar *spath, const NS_tchar *dpath, bool allowDirs = false) {

Thanks!

Pushed to mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/d84644c09199

Comment on attachment 8599421 [details] [diff] [review] patch Requesting now since I will be on pto soon Approval Request Comment [Feature/regressing bug #]: Bug 307181 [User impact if declined]: It is possible that some users may error during update with too many open files [Describe test coverage new/current, TreeHerder]: landed on m-i and soon to be on m-c. Tested locally. [Risks and why]: Minimal. This is an obvious correctness fix [String/UUID change made/needed]: None

Comment on attachment 8599421 [details] [diff] [review] patch [Triage Comment] Taking it in 38.0RC1 (because 38 is an ESR).

Robert, could you please provide some steps to verify this?

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #10) > Robert, could you please provide some steps to verify this? I mean verification on ESR 38 particularly.

This is a correctness fix that would take a significant effort to simulate a failure condition which is likely why we've never had a bug report for it previously. The patch landing, the tests passing, and updates being successful will likely need to be enough for verification.

Got it, thank you Robert! No manual verification for this then.