[WasmFS] Allow removeChild to return specific error codes, and use it in OPFS (#17793)

Author: kripken

src/library_wasmfs_opfs.js

@@ -202,10 +202,15 @@ mergeInto(LibraryManager.library, {
 
   _wasmfs_opfs_remove_child__deps: ['$wasmfsOPFSFree',
                                     '$wasmfsOPFSDirectoryHandles'],
-  _wasmfs_opfs_remove_child: async function(ctx, dirID, namePtr) {
+  _wasmfs_opfs_remove_child: async function(ctx, dirID, namePtr, errPtr) {
     let name = UTF8ToString(namePtr);
     let dirHandle = wasmfsOPFSDirectoryHandles.get(dirID);
-    await dirHandle.removeEntry(name);
+    try {
+      await dirHandle.removeEntry(name);
+    } catch {
+      let err = -{{{ cDefine('EIO') }}};
+      {{{ makeSetValue('errPtr', 0, 'err', 'i32') }}};
+    }
     _emscripten_proxy_finish(ctx);
   },
 

system/lib/wasmfs/backends/memory_backend.cpp

@@ -45,13 +45,13 @@ std::shared_ptr<File> MemoryDirectory::getChild(const std::string& name) {
   return nullptr;
 }
 
-bool MemoryDirectory::removeChild(const std::string& name) {
+int MemoryDirectory::removeChild(const std::string& name) {
   auto entry = findEntry(name);
   if (entry != entries.end()) {
     entry->child->locked().setParent(nullptr);
     entries.erase(entry);
   }
-  return true;
+  return 0;
 }
 
 std::vector<Directory::Entry> MemoryDirectory::getEntries() {
@@ -74,7 +74,7 @@ int MemoryDirectory::insertMove(const std::string& name,
       break;
     }
   }
-  removeChild(name);
+  (void)removeChild(name);
   insertChild(name, file);
   return 0;
 }

system/lib/wasmfs/backends/node_backend.cpp

@@ -217,19 +217,16 @@ class NodeDirectory : public Directory {
     }
   }
 
-  bool removeChild(const std::string& name) override {
+  int removeChild(const std::string& name) override {
     auto childPath = getChildPath(name);
     // Try both `unlink` and `rmdir`.
     if (auto err = _wasmfs_node_unlink(childPath.c_str())) {
       if (err == EISDIR) {
         err = _wasmfs_node_rmdir(childPath.c_str());
       }
-      if (err) {
-        // TODO: Report specific errors.
-        return false;
-      }
+      return -err;
     }
-    return true;
+    return 0;
   }
 
   std::shared_ptr<DataFile> insertDataFile(const std::string& name,

system/lib/wasmfs/backends/opfs_backend.cpp

@@ -47,7 +47,8 @@ void _wasmfs_opfs_move(em_proxying_ctx* ctx,
 
 void _wasmfs_opfs_remove_child(em_proxying_ctx* ctx,
                                int dir_id,
-                               const char* name);
+                               const char* name,
+                               int* err);
 
 void _wasmfs_opfs_get_entries(em_proxying_ctx* ctx,
                               int dirID,
@@ -411,11 +412,12 @@ class OPFSDirectory : public Directory {
     return err;
   }
 
-  bool removeChild(const std::string& name) override {
+  int removeChild(const std::string& name) override {
+    int err = 0;
     proxy([&](auto ctx) {
-      _wasmfs_opfs_remove_child(ctx.ctx, dirID, name.c_str());
+      _wasmfs_opfs_remove_child(ctx.ctx, dirID, name.c_str(), &err);
     });
-    return true;
+    return err;
   }
 
   size_t getNumEntries() override { return getEntries().size(); }

system/lib/wasmfs/file.cpp

@@ -192,23 +192,24 @@ int Directory::Handle::insertMove(const std::string& name,
   return 0;
 }
 
-bool Directory::Handle::removeChild(const std::string& name) {
+int Directory::Handle::removeChild(const std::string& name) {
   auto& dcache = getDir()->dcache;
   auto entry = dcache.find(name);
   // If this is a mount, we don't need to call into the backend.
   if (entry != dcache.end() && entry->second.kind == DCacheKind::Mount) {
     dcache.erase(entry);
-    return true;
+    return 0;
   }
-  if (!getDir()->removeChild(name)) {
-    return false;
+  if (auto err = getDir()->removeChild(name)) {
+    assert(err < 0);
+    return err;
   }
   if (entry != dcache.end()) {
     entry->second.file->locked().setParent(nullptr);
     dcache.erase(entry);
   }
   setMTime(time(NULL));
-  return true;
+  return 0;
 }
 
 std::string Directory::Handle::getName(std::shared_ptr<File> file) {

system/lib/wasmfs/file.h

@@ -208,10 +208,10 @@ class Directory : public File {
   virtual int insertMove(const std::string& name,
                          std::shared_ptr<File> file) = 0;
 
-  // Remove the file with the given name, returning `true` on success or if the
-  // child has already been removed or returning `false` if the child cannot be
-  // removed.
-  virtual bool removeChild(const std::string& name) = 0;
+  // Remove the file with the given name. Returns zero on success or if the
+  // child has already been removed and otherwise returns a negative error code
+  // if the child cannot be removed.
+  virtual int removeChild(const std::string& name) = 0;
 
   // The number of entries in this directory.
   virtual size_t getNumEntries() = 0;
@@ -377,10 +377,10 @@ class Directory::Handle : public File::Handle {
   [[nodiscard]] int insertMove(const std::string& name,
                                std::shared_ptr<File> file);
 
-  // Remove the file with the given name, returning `true` on success or if the
-  // vhild has already been removed or returning `false` if the child cannot be
-  // removed.
-  bool removeChild(const std::string& name);
+  // Remove the file with the given name. Returns zero on success or if the
+  // child has already been removed and otherwise returns a negative error code
+  // if the child cannot be removed.
+  [[nodiscard]] int removeChild(const std::string& name);
 
   std::string getName(std::shared_ptr<File> file);
 

system/lib/wasmfs/memory_backend.h

@@ -63,7 +63,7 @@ class MemoryDirectory : public Directory {
 
   std::shared_ptr<File> getChild(const std::string& name) override;
 
-  bool removeChild(const std::string& name) override;
+  int removeChild(const std::string& name) override;
 
   std::shared_ptr<DataFile> insertDataFile(const std::string& name,
                                            mode_t mode) override {

system/lib/wasmfs/syscalls.cpp

@@ -834,10 +834,7 @@ int __syscall_unlinkat(int dirfd, intptr_t path, int flags) {
   }
 
   // Input is valid, perform the unlink.
-  if (!lockedParent.removeChild(childName)) {
-    return -EPERM;
-  }
-  return 0;
+  return lockedParent.removeChild(childName);
 }
 
 int __syscall_rmdir(intptr_t path) {
@@ -1014,10 +1011,7 @@ int __syscall_renameat(int olddirfd,
   }
 
   // Perform the move.
-  if (auto err = lockedNewParent.insertMove(newFileName, oldFile)) {
-    return err;
-  }
-  return 0;
+  return lockedNewParent.insertMove(newFileName, oldFile);
 }
 
 int __syscall_rename(intptr_t oldpath, intptr_t newpath) {

test/wasmfs/wasmfs_opfs_errors.c

@@ -64,6 +64,19 @@ int try_truncate(void) {
   return 2;
 }
 
+EMSCRIPTEN_KEEPALIVE
+int try_unlink(void) {
+  int err = unlink(file);
+  if (err == 0) {
+    return 1;
+  }
+  if (errno == EIO) {
+    return 0;
+  }
+  emscripten_console_error(strerror(errno));
+  return 2;
+}
+
 EMSCRIPTEN_KEEPALIVE
 int try_oob_read(void) {
   int fd = open(file, O_RDWR);
@@ -74,12 +87,15 @@ int try_oob_read(void) {
   char buf;
   int nread = pread(fd, &buf, 1, (off_t)-1ll);
   if (nread > 0) {
+    close(fd);
     return 1;
   }
   if (errno == EINVAL) {
+    close(fd);
     return 0;
   }
   EM_ASM({ console.log('errno', $0); }, errno);
+  close(fd);
   return 2;
 }
 
@@ -93,12 +109,15 @@ int try_oob_write(void) {
   char buf = 0;
   int nread = pwrite(fd, &buf, 1, (off_t)-1ll);
   if (nread > 0) {
+    close(fd);
     return 1;
   }
   if (errno == EINVAL) {
+    close(fd);
     return 0;
   }
   emscripten_console_error(strerror(errno));
+  close(fd);
   return 2;
 }
 

test/wasmfs/wasmfs_opfs_errors_post.js

@@ -28,6 +28,10 @@ async function run_test() {
     throw "Did not get expected EIO when resizing file";
   }
 
+  if (Module._try_unlink() != 0) {
+    throw "Did not get expected EIO when unlinking file";
+  }
+
   await access.close();
 
   // We can open the file in any mode now that there is no open access
@@ -57,5 +61,10 @@ async function run_test() {
     throw "Did not get expected EINVAL doing out of bounds write";
   }
 
+  if (Module._try_unlink() != 1) {
+    throw "Did not succeed to unlink the file (which should work now that " +
+          "nothing prevents it)";
+  }
+
   Module._report_result(0);
 }