Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Gecko Bug 1791332] Add an OPFS-specific synchronous ordered cleanup queue #36182

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+54 −42
Open

[Gecko Bug 1791332] Add an OPFS-specific synchronous ordered cleanup queue #36182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 23 additions & 11 deletions fs/resources/test-helpers.js
View file
Open in desktop
Expand Up @@ -46,7 +46,7 @@ async function getSortedDirectoryEntries(handle) {

async function createDirectory(test, name, parent) {
const new_dir_handle = await parent.getDirectoryHandle(name, {create: true});
test.add_cleanup(async () => {
cleanup(test, new_dir_handle, async () => {
try {
await parent.removeEntry(name, {recursive: true});
} catch (e) {
Expand All @@ -59,7 +59,7 @@ async function createDirectory(test, name, parent) {

async function createEmptyFile(test, name, parent) {
const handle = await parent.getFileHandle(name, {create: true});
test.add_cleanup(async () => {
cleanup(test, handle, async () => {
try {
await parent.removeEntry(name);
} catch (e) {
Expand Down Expand Up @@ -87,20 +87,32 @@ function garbageCollect() {
self.gc();
};

var fs_cleanups = [];

async function cleanup(test, value, cleanup_func) {
test.add_cleanup(async () => {
try {
await cleanup_func();
} catch (e) {
// Ignore any errors when removing files, as tests might already remove
// the file.
}
});
if (fs_cleanups.length === 0) {
// register to get called back once from cleanup
test.add_cleanup(async () => {
// Cleanup in LIFO order to ensure locks are released correctly relative
// to thinks like removeEntry(). Do so in a serialized form, not in parallel!
fs_cleanups.reverse();
for (let cleanup of fs_cleanups) {
try {
await cleanup();
} catch (e) {
// Ignore any errors when removing files, as tests might already remove
// the file.
}
}
fs_cleanups.length = 0;
});
}
fs_cleanups.push(cleanup_func);
return value;
}

async function cleanup_writable(test, value) {
return cleanup(test, value, async () => {
value.close();
return value.close();
});
}
62 changes: 31 additions & 31 deletions fs/script-tests/FileSystemWritableFileStream-write.js
View file
Open in desktop
@@ -1,6 +1,6 @@
directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'empty_blob', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write(new Blob([]));
await stream.close();
Expand All @@ -11,7 +11,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'valid_blob', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write(new Blob(['1234567890']));
await stream.close();
Expand All @@ -22,7 +22,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'write_param_empty', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write({type: 'write', data: '1234567890'});
await stream.close();
Expand All @@ -33,7 +33,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'string_zero_offset', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write({type: 'write', position: 0, data: '1234567890'});
await stream.close();
Expand All @@ -44,7 +44,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'blob_zero_offset', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write({type: 'write', position: 0, data: new Blob(['1234567890'])});
await stream.close();
Expand All @@ -55,7 +55,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'write_appends', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write('12345');
await stream.write('67890');
Expand All @@ -67,7 +67,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'write_appends_object_string', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write('12345');
await stream.write({type: 'write', data: '67890'});
Expand All @@ -79,7 +79,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'write_appends_object_blob', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write('12345');
await stream.write({type: 'write', data: new Blob(['67890'])});
Expand All @@ -91,7 +91,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'string_with_offset', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write('1234567890');
await stream.write({type: 'write', position: 4, data: 'abc'});
Expand All @@ -103,7 +103,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'blob_with_offset', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write('1234567890');
await stream.write({type: 'write', position: 4, data: new Blob(['abc'])});
Expand All @@ -115,7 +115,7 @@ assert_equals(await getFileSize(handle), 10);

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'bad_offset', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write({type: 'write', position: 4, data: new Blob(['abc'])});
await stream.close();
Expand All @@ -126,7 +126,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'empty_string', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write('');
await stream.close();
Expand All @@ -136,7 +136,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'valid_utf8_string', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write('foo🤘');
await stream.close();
Expand All @@ -146,7 +146,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'string_with_unix_line_ending', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write('foo\n');
await stream.close();
Expand All @@ -157,7 +157,7 @@ directory_test(async (t, root) => {
directory_test(async (t, root) => {
const handle =
await createEmptyFile(t, 'string_with_windows_line_ending', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await stream.write('foo\r\n');
await stream.close();
Expand All @@ -167,7 +167,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'empty_array_buffer', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

const buf = new ArrayBuffer(0);
await stream.write(buf);
Expand All @@ -179,7 +179,7 @@ directory_test(async (t, root) => {
directory_test(async (t, root) => {
const handle =
await createEmptyFile(t, 'valid_string_typed_byte_array', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

const buf = new ArrayBuffer(3);
const intView = new Uint8Array(buf);
Expand All @@ -196,7 +196,7 @@ directory_test(async (t, root) => {
const dir = await createDirectory(t, 'parent_dir', root);
const file_name = 'close_fails_when_dir_removed.txt';
const handle = await createEmptyFile(t, file_name, dir);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());
await stream.write('foo');

await root.removeEntry('parent_dir', {recursive: true});
Expand All @@ -205,10 +205,10 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'atomic_writes.txt', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());
await stream.write('foox');

const stream2 = await handle.createWritable();
const stream2 = await cleanup_writable(t, await handle.createWritable());
await stream2.write('bar');

assert_equals(await getFileSize(handle), 0);
Expand All @@ -224,7 +224,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'atomic_write_after_close.txt', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());
await stream.write('foo');

await stream.close();
Expand All @@ -238,7 +238,7 @@ directory_test(async (t, root) => {
directory_test(async (t, root) => {
const handle =
await createEmptyFile(t, 'atomic_truncate_after_close.txt', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());
await stream.write('foo');

await stream.close();
Expand All @@ -250,7 +250,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'atomic_close_after_close.txt', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());
await stream.write('foo');

await stream.close();
Expand All @@ -262,7 +262,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'there_can_be_only_one.txt', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());
await stream.write('foo');

// This test might be flaky if there is a race condition allowing
Expand All @@ -279,7 +279,7 @@ directory_test(async (t, root) => {
const file_name = 'atomic_writable_file_stream_persists_removed.txt';
const handle = await createFileWithContents(t, file_name, 'foo', dir);

const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());
await stream.write('bar');

await dir.removeEntry(file_name);
Expand All @@ -292,7 +292,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'writer_written', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());
assert_false(stream.locked);
const writer = stream.getWriter();
assert_true(stream.locked);
Expand All @@ -310,7 +310,7 @@ directory_test(async (t, root) => {
directory_test(async (t, root) => {
const handle = await createFileWithContents(
t, 'content.txt', 'very long string', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await promise_rejects_dom(
t, "SyntaxError", stream.write({type: 'truncate'}), 'truncate without size');
Expand All @@ -319,7 +319,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'content.txt', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await promise_rejects_dom(
t, "SyntaxError", stream.write({type: 'write'}), 'write without data');
Expand All @@ -328,7 +328,7 @@ directory_test(async (t, root) => {

directory_test(async (t, root) => {
const handle = await createEmptyFile(t, 'content.txt', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await promise_rejects_js(
t, TypeError, stream.write({type: 'write', data: null}), 'write with null data');
Expand All @@ -338,7 +338,7 @@ directory_test(async (t, root) => {
directory_test(async (t, root) => {
const handle = await createFileWithContents(
t, 'content.txt', 'seekable', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());

await promise_rejects_dom(
t, "SyntaxError", stream.write({type: 'seek'}), 'seek without position');
Expand All @@ -352,7 +352,7 @@ directory_test(async (t, root) => {
await root.removeEntry(source_file.name);

const handle = await createEmptyFile(t, 'invalid_blob_test', root);
const stream = await handle.createWritable();
const stream = await cleanup_writable(t, await handle.createWritable());
await promise_rejects_dom(t, "NotFoundError", stream.write(source_blob));
await promise_rejects_js(t, TypeError, stream.close());

Expand Down