Bug 1725615 - Port osfile.jsm usage to IOUtils in toolkit/crashreporter/ r=gsvelto

Differential Revision: https://phabricator.services.mozilla.com/D163057
This commit is contained in:
Barret Rennie 2023-02-17 06:34:30 +00:00
parent b59f2a6b1f
commit 019daed1af
8 changed files with 131 additions and 179 deletions

View File

@ -6,10 +6,6 @@ const { FileUtils } = ChromeUtils.importESModule(
"resource://gre/modules/FileUtils.sys.mjs"
);
const lazy = {};
ChromeUtils.defineModuleGetter(lazy, "OS", "resource://gre/modules/osfile.jsm");
var EXPORTED_SYMBOLS = ["CrashSubmit"];
const SUCCESS = "success";
@ -36,16 +32,16 @@ function parseINIStrings(path) {
// Since we're basically re-implementing (with async) part of the crashreporter
// client here, we'll just steal the strings we need from crashreporter.ini
async function getL10nStrings() {
let path = lazy.OS.Path.join(
let path = PathUtils.join(
Services.dirsvc.get("GreD", Ci.nsIFile).path,
"crashreporter.ini"
);
let pathExists = await lazy.OS.File.exists(path);
let pathExists = await IOUtils.exists(path);
if (!pathExists) {
// we if we're on a mac
let parentDir = lazy.OS.Path.dirname(path);
path = lazy.OS.Path.join(
let parentDir = PathUtils.parent(path);
path = PathUtils.join(
parentDir,
"MacOS",
"crashreporter.app",
@ -54,7 +50,7 @@ async function getL10nStrings() {
"crashreporter.ini"
);
let pathExists = await lazy.OS.File.exists(path);
let pathExists = await IOUtils.exists(path);
if (!pathExists) {
// This happens on Android where everything is in an APK.
@ -73,11 +69,11 @@ async function getL10nStrings() {
reporturl: crstrings.CrashDetailsURL,
};
path = lazy.OS.Path.join(
path = PathUtils.join(
Services.dirsvc.get("XCurProcD", Ci.nsIFile).path,
"crashreporter-override.ini"
);
pathExists = await lazy.OS.File.exists(path);
pathExists = await IOUtils.exists(path);
if (pathExists) {
crstrings = parseINIStrings(path);
@ -96,24 +92,21 @@ async function getL10nStrings() {
function getDir(name) {
let uAppDataPath = Services.dirsvc.get("UAppData", Ci.nsIFile).path;
return lazy.OS.Path.join(uAppDataPath, "Crash Reports", name);
return PathUtils.join(uAppDataPath, "Crash Reports", name);
}
async function writeFileAsync(dirName, fileName, data) {
let dirPath = getDir(dirName);
let filePath = lazy.OS.Path.join(dirPath, fileName);
// succeeds even with existing path, permissions 700
await lazy.OS.File.makeDir(dirPath, {
unixFlags: lazy.OS.Constants.libc.S_IRWXU,
});
await lazy.OS.File.writeAtomic(filePath, data, { encoding: "utf-8" });
let filePath = PathUtils.join(dirPath, fileName);
await IOUtils.makeDirectory(dirPath, { permissions: 0o700 });
await IOUtils.writeUTF8(filePath, data);
}
function getPendingMinidump(id) {
let pendingDir = getDir("pending");
return [".dmp", ".extra", ".memory.json.gz"].map(suffix => {
return lazy.OS.Path.join(pendingDir, `${id}${suffix}`);
return PathUtils.join(pendingDir, `${id}${suffix}`);
});
}
@ -160,7 +153,7 @@ Submitter.prototype = {
await Promise.all(
toDelete.map(path => {
return lazy.OS.File.remove(path, { ignoreAbsent: true });
return IOUtils.remove(path, { ignoreAbsent: true });
})
);
} catch (ex) {
@ -355,9 +348,7 @@ Submitter.prototype = {
"TelemetrySessionId",
"TelemetryServerURL",
];
let decoder = new TextDecoder();
let extraData = await lazy.OS.File.read(extra);
let extraKeyVals = JSON.parse(decoder.decode(extraData));
let extraKeyVals = await IOUtils.readJSON(extra);
this.extraKeyVals = { ...extraKeyVals, ...this.extraKeyVals };
strippedAnnotations.forEach(key => delete this.extraKeyVals[key]);
@ -370,9 +361,9 @@ Submitter.prototype = {
let [dump, extra, memory] = getPendingMinidump(this.id);
let [dumpExists, extraExists, memoryExists] = await Promise.all([
lazy.OS.File.exists(dump),
lazy.OS.File.exists(extra),
lazy.OS.File.exists(memory),
IOUtils.exists(dump),
IOUtils.exists(extra),
IOUtils.exists(memory),
]);
if (!dumpExists || !extraExists) {
@ -397,7 +388,7 @@ Submitter.prototype = {
this.id + "-" + name
);
dumpsExistsPromises.push(lazy.OS.File.exists(dump));
dumpsExistsPromises.push(IOUtils.exists(dump));
additionalDumps.push({ name, dump });
}
@ -501,7 +492,7 @@ var CrashSubmit = {
delete: async function CrashSubmit_delete(id) {
await Promise.all(
getPendingMinidump(id).map(path => {
return lazy.OS.File.remove(path, { ignoreAbsent: true });
return IOUtils.remove(path);
})
);
},
@ -518,12 +509,8 @@ var CrashSubmit = {
*/
ignore: async function CrashSubmit_ignore(id) {
let [dump /* , extra, memory */] = getPendingMinidump(id);
let file = await lazy.OS.File.open(
`${dump}.ignore`,
{ create: true },
{ unixFlags: lazy.OS.Constants.libc.O_CREAT }
);
await file.close();
const ignorePath = `${dump}.ignore`;
await IOUtils.writeUTF8(ignorePath, "", { mode: "create" });
},
/**
@ -536,41 +523,41 @@ var CrashSubmit = {
*/
pendingIDs: async function CrashSubmit_pendingIDs(minFileDate) {
let ids = [];
let dirIter = null;
let pendingDir = getDir("pending");
if (!(await IOUtils.exists(pendingDir))) {
return ids;
}
let children;
try {
dirIter = new lazy.OS.File.DirectoryIterator(pendingDir);
children = await IOUtils.getChildren(pendingDir);
} catch (ex) {
console.error(ex);
throw ex;
}
if (!(await dirIter.exists())) {
return ids;
}
try {
let entries = Object.create(null);
let ignored = Object.create(null);
const entries = Object.create(null);
const ignored = Object.create(null);
// `await` in order to ensure all callbacks are called
await dirIter.forEach(entry => {
if (!entry.isDir /* is file */) {
let matches = entry.name.match(/(.+)\.dmp$/);
for (const child of children) {
const info = await IOUtils.stat(child);
if (info.type !== "directory") {
const name = PathUtils.filename(child);
const matches = name.match(/(.+)\.dmp$/);
if (matches) {
let id = matches[1];
const id = matches[1];
if (UUID_REGEX.test(id)) {
entries[id] = lazy.OS.File.stat(entry.path);
entries[id] = info;
}
} else {
// maybe it's a .ignore file
let matchesIgnore = entry.name.match(/(.+)\.dmp.ignore$/);
const matchesIgnore = name.match(/(.+)\.dmp.ignore$/);
if (matchesIgnore) {
let id = matchesIgnore[1];
const id = matchesIgnore[1];
if (UUID_REGEX.test(id)) {
ignored[id] = true;
@ -578,20 +565,17 @@ var CrashSubmit = {
}
}
}
});
}
for (let entry in entries) {
let entryInfo = await entries[entry];
if (!(entry in ignored) && entryInfo.lastAccessDate > minFileDate) {
ids.push(entry);
for (const [id, info] of Object.entries(entries)) {
const accessDate = new Date(info.lastAccessed);
if (!(id in ignored) && accessDate > minFileDate) {
ids.push(id);
}
}
} catch (ex) {
console.error(ex);
throw ex;
} finally {
dirIter.close();
}
return ids;
@ -606,45 +590,43 @@ var CrashSubmit = {
pruneSavedDumps: async function CrashSubmit_pruneSavedDumps() {
const KEEP = 10;
let dirIter = null;
let dirEntries = [];
let pendingDir = getDir("pending");
let children;
try {
dirIter = new lazy.OS.File.DirectoryIterator(pendingDir);
children = await IOUtils.getChildren(pendingDir);
} catch (ex) {
if (ex instanceof lazy.OS.File.Error && ex.becauseNoSuchFile) {
if (DOMException.isInstance(ex) && ex.name === "NotFoundError") {
return [];
}
throw ex;
}
try {
await dirIter.forEach(entry => {
if (!entry.isDir /* is file */) {
let matches = entry.name.match(/(.+)\.extra$/);
for (const path of children) {
let infoPromise;
try {
infoPromise = IOUtils.stat(path);
} catch (ex) {
console.error(ex);
throw ex;
}
if (matches) {
dirEntries.push({
name: entry.name,
path: entry.path,
// dispatch promise instead of blocking iteration on `await`
infoPromise: lazy.OS.File.stat(entry.path),
});
}
}
});
} catch (ex) {
console.error(ex);
throw ex;
} finally {
dirIter.close();
const name = PathUtils.filename(path);
if (name.match(/(.+)\.extra$/)) {
dirEntries.push({
name,
path,
infoPromise,
});
}
}
dirEntries.sort(async (a, b) => {
let dateA = (await a.infoPromise).lastModificationDate;
let dateB = (await b.infoPromise).lastModificationDate;
let dateA = (await a.infoPromise).lastModified;
let dateB = (await b.infoPromise).lastModified;
if (dateA < dateB) {
return -1;
@ -665,9 +647,9 @@ var CrashSubmit = {
let matches = extra.leafName.match(/(.+)\.extra$/);
if (matches) {
let pathComponents = lazy.OS.Path.split(extra.path);
let pathComponents = PathUtils.split(extra.path);
pathComponents[pathComponents.length - 1] = matches[1];
let path = lazy.OS.Path.join(...pathComponents);
let path = PathUtils.join(...pathComponents);
toDelete.push(extra.path, `${path}.dmp`, `${path}.memory.json.gz`);
}
@ -675,7 +657,7 @@ var CrashSubmit = {
await Promise.all(
toDelete.map(path => {
return lazy.OS.File.remove(path, { ignoreAbsent: true });
return IOUtils.remove(path, { ignoreAbsent: true });
})
);
}

View File

@ -7,7 +7,6 @@ let reportURL;
const { CrashReports } = ChromeUtils.import(
"resource://gre/modules/CrashReports.jsm"
);
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
ChromeUtils.defineModuleGetter(
this,
@ -190,8 +189,8 @@ async function clearUnsubmittedReports() {
return;
}
await cleanupFolder(CrashReports.pendingDir.path);
await clearOldReports();
await enqueueCleanup(() => cleanupFolder(CrashReports.pendingDir.path));
await enqueueCleanup(clearOldReports);
document.getElementById("reportListUnsubmitted").classList.add("hidden");
}
@ -224,11 +223,13 @@ async function clearSubmittedReports() {
return;
}
await cleanupFolder(
CrashReports.submittedDir.path,
async entry => entry.name.startsWith("bp-") && entry.name.endsWith(".txt")
await enqueueCleanup(async () =>
cleanupFolder(
CrashReports.submittedDir.path,
async entry => entry.name.startsWith("bp-") && entry.name.endsWith(".txt")
)
);
await clearOldReports();
await enqueueCleanup(clearOldReports);
document.getElementById("reportListSubmitted").classList.add("hidden");
document.getElementById("noSubmittedReports").classList.remove("hidden");
}
@ -246,12 +247,8 @@ async function clearOldReports() {
return false;
}
let date = entry.winLastWriteDate;
if (!date) {
const stat = await OS.File.stat(entry.path);
date = stat.lastModificationDate;
}
return date < oneYearAgo;
const stat = await IOUtils.stat(entry.path);
return stat.lastModified < oneYearAgo;
});
}
@ -264,19 +261,25 @@ async function clearOldReports() {
* returning whether to delete the file
*/
async function cleanupFolder(path, filter) {
const iterator = new OS.File.DirectoryIterator(path);
function entry(path) {
return {
path,
name: PathUtils.filename(path),
};
}
let children;
try {
await iterator.forEach(async entry => {
if (!filter || (await filter(entry))) {
await OS.File.remove(entry.path);
}
});
children = await IOUtils.getChildren(path);
} catch (e) {
if (!(e instanceof OS.File.Error) || !e.becauseNoSuchFile) {
if (DOMException.isInstance(e) || e.name !== "NotFoundError") {
throw e;
}
} finally {
iterator.close();
}
for (const childPath of children) {
if (!filter || (await filter(entry(childPath)))) {
await IOUtils.remove(childPath);
}
}
}
@ -290,3 +293,26 @@ function dispatchEvent(name) {
event.initEvent(name, true, false);
document.dispatchEvent(event);
}
let cleanupQueue = Promise.resolve();
/**
* Enqueue a cleanup function.
*
* Instead of directly calling cleanup functions as a result of DOM
* interactions, queue them through this function so that we do not have
* overlapping executions of cleanup functions.
*
* Cleanup functions overlapping could cause a race where one function is
* attempting to stat a file while another function is attempting to delete it,
* causing an exception.
*
* @param fn The cleanup function to call. It will be called once the last
* cleanup function has resolved.
*
* @returns A promise to await instead of awaiting the cleanup function.
*/
function enqueueCleanup(fn) {
cleanupQueue = cleanupQueue.then(fn);
return cleanupQueue;
}

View File

@ -1,4 +1,3 @@
var { OS, require } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
const { makeFakeAppDir } = ChromeUtils.importESModule(
"resource://testing-common/AppData.sys.mjs"
);
@ -7,7 +6,7 @@ var { AppConstants } = ChromeUtils.importESModule(
);
function getEventDir() {
return OS.Path.join(do_get_tempdir().path, "crash-events");
return PathUtils.join(do_get_tempdir().path, "crash-events");
}
function sendCommandAsync(command) {
@ -152,9 +151,7 @@ async function handleMinidump(callback) {
registerCleanupFunction(cleanup);
Assert.ok(extrafile.exists());
let data = await OS.File.read(extrafile.path);
let decoder = new TextDecoder();
let extra = JSON.parse(decoder.decode(data));
let extra = await IOUtils.readJSON(extrafile.path);
if (callback) {
await callback(minidump, extra, extrafile, memoryfile);

View File

@ -198,9 +198,7 @@ async function do_x64CFITest(how, expectedStack, minidumpAnalyzerArgs) {
runMinidumpAnalyzer(minidumpFile, minidumpAnalyzerArgs);
// Refresh updated extra data
let data = await OS.File.read(extraFile.path);
let decoder = new TextDecoder();
extra = JSON.parse(decoder.decode(data));
extra = await IOUtils.readJSON(extraFile.path);
initTestCrasherSymbols();
let stackTraces = extra.StackTraces;

View File

@ -40,23 +40,19 @@ function after_crash(mdump, extra) {
Assert.equal(data.conditions[0].filename, "-e");
}
// Test that AsyncShutdown + OS.File reports errors correctly, in a case in which
// the latest operation succeeded
// Test that AsyncShutdown + IOUtils reports errors correctly.,
function setup_osfile_crash_noerror() {
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
function setup_ioutils_crash() {
const { PromiseUtils } = ChromeUtils.importESModule(
"resource://gre/modules/PromiseUtils.sys.mjs"
);
Services.prefs.setIntPref("toolkit.asyncshutdown.crash_timeout", 1);
Services.prefs.setBoolPref("toolkit.osfile.native", false);
OS.File.profileBeforeChange.addBlocker(
IOUtils.profileBeforeChange.addBlocker(
"Adding a blocker that will never be resolved",
() => PromiseUtils.defer().promise
);
OS.File.getCurrentDirectory();
Services.startup.advanceShutdownPhase(
Services.startup.SHUTDOWN_PHASE_APPSHUTDOWN
@ -64,57 +60,16 @@ function setup_osfile_crash_noerror() {
dump("Waiting for crash\n");
}
function after_osfile_crash_noerror(mdump, extra) {
info("after OS.File crash: " + extra.AsyncShutdownTimeout);
function after_ioutils_crash(mdump, extra) {
info("after IOUtils crash: " + extra.AsyncShutdownTimeout);
let data = JSON.parse(extra.AsyncShutdownTimeout);
let state = data.conditions[0].state;
info("Keys: " + Object.keys(state).join(", "));
Assert.equal(data.phase, "profile-before-change");
Assert.ok(state.launched);
Assert.ok(!state.shutdown);
Assert.ok(state.worker);
Assert.ok(!!state.latestSent);
Assert.equal(state.latestSent[1], "getCurrentDirectory");
}
// Test that AsyncShutdown + OS.File reports errors correctly, in a case in which
// the latest operation failed
function setup_osfile_crash_exn() {
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
const { PromiseUtils } = ChromeUtils.importESModule(
"resource://gre/modules/PromiseUtils.sys.mjs"
Assert.equal(
data.phase,
"IOUtils: waiting for profileBeforeChange IO to complete"
);
Services.prefs.setIntPref("toolkit.asyncshutdown.crash_timeout", 1);
Services.prefs.setBoolPref("toolkit.osfile.native", false);
OS.File.profileBeforeChange.addBlocker(
"Adding a blocker that will never be resolved",
() => PromiseUtils.defer().promise
);
OS.File.read("I do not exist");
Services.startup.advanceShutdownPhase(
Services.startup.SHUTDOWN_PHASE_APPSHUTDOWN
);
dump("Waiting for crash\n");
}
function after_osfile_crash_exn(mdump, extra) {
info("after OS.File crash: " + extra.AsyncShutdownTimeout);
let data = JSON.parse(extra.AsyncShutdownTimeout);
let state = data.conditions[0].state;
info("Keys: " + Object.keys(state).join(", "));
Assert.equal(data.phase, "profile-before-change");
Assert.ok(!state.shutdown);
Assert.ok(state.worker);
Assert.ok(!!state.latestSent);
Assert.equal(state.latestSent[1], "read");
}
add_task(async function run_test() {
await do_crash(setup_crash, after_crash);
await do_crash(setup_osfile_crash_noerror, after_osfile_crash_noerror);
await do_crash(setup_osfile_crash_exn, after_osfile_crash_exn);
await do_crash(setup_ioutils_crash, after_ioutils_crash);
});

View File

@ -16,9 +16,7 @@ add_task(async function run_test() {
runMinidumpAnalyzer(mdump);
// Refresh updated extra data
let data = await OS.File.read(extraFile.path);
let decoder = new TextDecoder();
extra = JSON.parse(decoder.decode(data));
extra = await IOUtils.readJSON(extraFile.path);
Assert.equal(
extra.StackTraces.crash_info.type,

View File

@ -16,9 +16,7 @@ add_task(async function run_test() {
runMinidumpAnalyzer(mdump);
// Refresh updated extra data
let data = await OS.File.read(extraFile.path);
let decoder = new TextDecoder();
extra = JSON.parse(decoder.decode(data));
extra = await IOUtils.readJSON(extraFile.path);
Assert.equal(extra.StackTraces.crash_info.type, "STATUS_HEAP_CORRUPTION");
Assert.equal(extra.TestKey, "TestValue");

View File

@ -19,9 +19,7 @@ add_task(async function run_test() {
runMinidumpAnalyzer(mdump);
// Refresh updated extra data
let data = await OS.File.read(extraFile.path);
let decoder = new TextDecoder();
extra = JSON.parse(decoder.decode(data));
extra = await IOUtils.readJSON(extraFile.path);
// Check unloaded modules
const unloadedModules = extra.StackTraces.unloaded_modules;