Fix memory and resource leaks in failing resource provider open callbacks
When an mjpResourceProvider's open callback returns 0 (failure), MuJoCo does not invoke the corresponding close callback. Previously, several resource provider implementations allocated heap memory or system file handles before encountering an error, failing to clean them up before returning 0. This change addresses these leaks across the codebase and clarifies the API contract. PiperOrigin-RevId: 929803942 Change-Id: Ib7344033726895cdf037a8d9356ffc183c78c2d3
This commit is contained in:
committed by
Copybara-Service
parent
c31e94cee6
commit
edbe6a6f74
@@ -1871,7 +1871,10 @@ mjfOpenResource
|
||||
|
||||
typedef int (*mjfOpenResource)(mjResource* resource);
|
||||
|
||||
This callback is for opening a resource; returns zero on failure.
|
||||
This callback is for opening a resource; returns zero on failure. Note that
|
||||
if this callback returns zero, the ``close`` callback will not be called.
|
||||
Therefore, the ``open`` callback is responsible for cleaning up any allocated
|
||||
memory or resources before returning zero to avoid memory leaks.
|
||||
|
||||
.. _mjfReadResource:
|
||||
|
||||
|
||||
@@ -33,7 +33,10 @@ struct mjResource_ {
|
||||
};
|
||||
typedef struct mjResource_ mjResource;
|
||||
|
||||
// callback for opening a resource, returns zero on failure
|
||||
// callback for opening a resource, returns zero on failure.
|
||||
// Note: If opening fails, the close callback will not be called. Therefore, the
|
||||
// open callback is responsible for cleaning up any allocated memory before
|
||||
// returning 0.
|
||||
typedef int (*mjfOpenResource)(mjResource* resource);
|
||||
|
||||
// callback for reading a resource
|
||||
|
||||
@@ -79,6 +79,10 @@ class Viewer {
|
||||
resource_provider.open = [](mjResource* resource) {
|
||||
auto* data = new ResourceData();
|
||||
data->bytes = LoadAsset(resource->name);
|
||||
if (data->bytes.empty()) {
|
||||
delete data;
|
||||
return 0;
|
||||
}
|
||||
resource->data = data;
|
||||
return static_cast<int>(data->bytes.size());
|
||||
};
|
||||
@@ -135,9 +139,7 @@ class Viewer {
|
||||
bytes_per_pixel);
|
||||
}
|
||||
|
||||
std::string GetDropFile() {
|
||||
return window_->GetDropFile();
|
||||
}
|
||||
std::string GetDropFile() { return window_->GetDropFile(); }
|
||||
|
||||
void Present(const mujoco::python::MjModelWrapper& model,
|
||||
mujoco::python::MjDataWrapper& data,
|
||||
|
||||
@@ -84,6 +84,10 @@ int main(int argc, char** argv, char** envp) {
|
||||
resource_provider.open = [](mjResource* resource) {
|
||||
const std::string resolved_path = Resolve(resource->name);
|
||||
FileResource* f = new FileResource(resolved_path);
|
||||
if (f->Size() == 0) {
|
||||
delete f;
|
||||
return 0;
|
||||
}
|
||||
resource->data = f;
|
||||
return f->Size();
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user