diff --git a/src/user/user_vfs.cc b/src/user/user_vfs.cc index 44c82137..b88667f5 100644 --- a/src/user/user_vfs.cc +++ b/src/user/user_vfs.cc @@ -105,7 +105,9 @@ std::string StripPathAndLower(std::string path) { namespace mujoco::user { -VFS::VFS(mjVFS* vfs) : self_(vfs) { +VFS::VFS(mjVFS* vfs) : owner_(vfs) { + stable_vfs_.impl_ = this; + mjp_defaultResourceProvider(&default_provider_); default_provider_.open = [](mjResource* res) { return OpenFile(res->name, res); @@ -121,7 +123,7 @@ VFS::VFS(mjVFS* vfs) : self_(vfs) { }; default_provider_.prefix = nullptr; - default_mount_.vfs = self_; + default_mount_.vfs = &stable_vfs_; default_mount_.provider = &default_provider_; default_mount_.data = nullptr; default_mount_.name = nullptr; @@ -245,7 +247,7 @@ int VFS::Read(mjResource* resource, const void** buffer) { VFS::ResourcePtr VFS::CreateResource(std::string_view name, const mjpResourceProvider* provider) { mjResource* res = new mjResource(); - res->vfs = self_; + res->vfs = &stable_vfs_; res->provider = provider; res->data = nullptr; res->name = new char[name.size() + 1]; @@ -312,20 +314,31 @@ mjResource* VFS::FindMount(const std::string& fullpath) { void VFS::MaybeSelfDestruct() { if (destructor_) { - destructor_(self_); + destructor_(owner_); } } +void VFS::Bind(mjVFS* vfs) { + owner_ = vfs; +} + void VFS::SetToSelfDestruct(std::function destructor) { destructor_ = std::move(destructor); } VFS* VFS::Upcast(mjVFS* vfs) { - return vfs ? static_cast(vfs->impl_) : nullptr; + if (!vfs) { + return nullptr; + } + VFS* impl = static_cast(vfs->impl_); + if (impl) { + impl->Bind(vfs); + } + return impl; } const VFS* VFS::Upcast(const mjVFS* vfs) { - return vfs ? static_cast(vfs->impl_) : nullptr; + return Upcast(const_cast(vfs)); } } // namespace mujoco::user diff --git a/src/user/user_vfs.h b/src/user/user_vfs.h index b8d8013f..4f5ebc26 100644 --- a/src/user/user_vfs.h +++ b/src/user/user_vfs.h @@ -117,7 +117,12 @@ class VFS { // that `this` will be invalidated after this call. void MaybeSelfDestruct(); - mjVFS* self_; + // Rebinds the current public mjVFS pointer to this implementation. + // This supports scenarios where the public mjVFS struct is moved. + void Bind(mjVFS* vfs); + + mjVFS stable_vfs_; + mjVFS* owner_; std::mutex mutex_; // Protects open_resources_ and mounts_. std::unordered_map open_resources_; std::unordered_map mounts_; diff --git a/test/user/user_vfs_test.cc b/test/user/user_vfs_test.cc index 84d750c3..f59fe6c4 100644 --- a/test/user/user_vfs_test.cc +++ b/test/user/user_vfs_test.cc @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -286,6 +287,29 @@ TEST_F(UserVfsTest, Timestamps) { mj_deleteVFS(&vfs); } +TEST_F(UserVfsTest, MoveVfsAfterOpenResource) { + mjVFS vfs; + mj_defaultVFS(&vfs); + + std::string buffer = ""; + mj_addBufferVFS(&vfs, "model", static_cast(buffer.c_str()), + buffer.size()); + + mjResource* resource = mju_openResource("", "model", &vfs, nullptr, 0); + ASSERT_THAT(resource, NotNull()); + + // Move the public mjVFS object after resources have been opened. + mjVFS moved = std::move(vfs); + + const void* out = nullptr; + const int size = mju_readResource(resource, &out); + EXPECT_GT(size, 0); + EXPECT_THAT(out, NotNull()); + + mju_closeResource(resource); + mj_deleteVFS(&moved); +} + TEST_F(UserVfsTest, MountUnmount) { int test = 0; int expect = 0;