Skip to content

Commit 376c6ff

Browse files
authored
HandleAllocator: Always reserve handle zero. NFC (#19065)
Also, set entries to `undefined` rather than using the `delete` operator on the handle array. Apparently using `delete` can lead to holes in the array which can damage perf. Reserving 0 also allows us to simplify the allocate function a little. This actually revealed a hidden bug in test/other/test_dlopen_promise.c. Split out from #19054
1 parent 6026829 commit 376c6ff

File tree

5 files changed

+13
-15
lines changed

5 files changed

+13
-15
lines changed

src/library.js

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3654,30 +3654,27 @@ mergeInto(LibraryManager.library, {
36543654

36553655
$HandleAllocator__docs: '/** @constructor */',
36563656
$HandleAllocator: function() {
3657-
this.allocated = [];
3657+
// Reserve slot 0 so that 0 is always and invalid handle
3658+
this.allocated = [undefined];
36583659
this.freelist = [];
36593660
this.get = function(id) {
36603661
#if ASSERTIONS
3661-
assert(this.allocated[id] !== undefined);
3662+
assert(this.allocated[id] !== undefined, 'invalid handle: ' + id);
36623663
#endif
36633664
return this.allocated[id];
36643665
};
36653666
this.allocate = function(handle) {
3666-
let id;
3667-
if (this.freelist.length > 0) {
3668-
id = this.freelist.pop();
3669-
this.allocated[id] = handle;
3670-
} else {
3671-
id = this.allocated.length;
3672-
this.allocated.push(handle);
3673-
}
3667+
let id = this.freelist.pop() || this.allocated.length;
3668+
this.allocated[id] = handle;
36743669
return id;
36753670
};
36763671
this.free = function(id) {
36773672
#if ASSERTIONS
36783673
assert(this.allocated[id] !== undefined);
36793674
#endif
3680-
delete this.allocated[id];
3675+
// Set the slot to `undefined` rather than using `delete` here since
3676+
// apparently arrays with holes in them can be less efficient.
3677+
this.allocated[id] = undefined;
36813678
this.freelist.push(id);
36823679
};
36833680
},

src/library_wasmfs_opfs.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ mergeInto(LibraryManager.library, {
1616

1717
_wasmfs_opfs_init_root_directory__deps: ['$wasmfsOPFSDirectoryHandles'],
1818
_wasmfs_opfs_init_root_directory: async function(ctx) {
19-
if (wasmfsOPFSDirectoryHandles.allocated.length == 0) {
19+
// allocated.length start of as 1 since 0 is a reserved handle
20+
if (wasmfsOPFSDirectoryHandles.allocated.length == 1) {
2021
// Directory 0 is reserved as the root
2122
let root = await navigator.storage.getDirectory();
2223
wasmfsOPFSDirectoryHandles.allocated.push(root);

system/lib/wasmfs/backends/opfs_backend.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ class OPFSBackend : public Backend {
456456

457457
std::shared_ptr<Directory> createDirectory(mode_t mode) override {
458458
proxy([](auto ctx) { _wasmfs_opfs_init_root_directory(ctx.ctx); });
459-
return std::make_shared<OPFSDirectory>(mode, this, 0, proxy);
459+
return std::make_shared<OPFSDirectory>(mode, this, 1, proxy);
460460
}
461461

462462
std::shared_ptr<Symlink> createSymlink(std::string target) override {

test/core/test_promise.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ static em_promise_result_t finish(void** result, void* data, void* value) {
253253

254254
// We should not have leaked any handles.
255255
EM_ASM({
256-
promiseMap.allocated.forEach(() => assert(false, "non-destroyed handle"));
256+
promiseMap.allocated.forEach((p) => assert(typeof p === "undefined", "non-destroyed handle"));
257257
});
258258

259259
// Cannot exit directly in a promise callback, since it would end up rejecting

test/other/test_dlopen_promise.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ em_promise_result_t on_rejected(void **result, void* data, void *value) {
2222

2323
int main() {
2424
em_promise_t inner = emscripten_dlopen_promise("libside.so", RTLD_NOW);
25-
em_promise_t outer = emscripten_promise_then(outer, on_fullfilled, on_rejected, NULL);
25+
em_promise_t outer = emscripten_promise_then(inner, on_fullfilled, on_rejected, NULL);
2626
emscripten_promise_destroy(outer);
2727
emscripten_promise_destroy(inner);
2828
printf("returning from main\n");

0 commit comments

Comments
 (0)