The memory is leaked when FSampler2D is dropped about gli-rs HOT 8 CLOSED

A newer version of this crate has been published to crates.io.
It should fix the problem of this issue.
If the problem still exists, reopen this issue.

from gli-rs.

That is probably a memory bug of my code and also the difficult part of writing this crate.
In deed, Drop is not implemented for exposed texture struct like Texture2D, FSampler2D.
Instead I implement Drop for the ffi texture base struct(which is a shared member of all exposed texture structs):

In the original C++ implementation, the fsampler2D is just a plain struct(so there no memory problem on it), but the texture base class hold a shared pointer to the actual texture data. This shared pointer may increase when creating sub image class like GliImage by Texture2D::get_level() method.

And I implement this crate just for loading small amount of ktx texture, so I have not tested the case of heavy loading. I had written some code to confirm that the shared pointer counter would decrease when the above drop method is called. However further investigation about memory consumption have not been fully tested yet. Maybe the destructor of Texture class in C++ is not called correctly even the shared pointer counter has been decreased to 0.

from gli-rs.

Could you provide the program to reproduce this problem(just a test problem to eat out the memory using this crate)? It would be helpful for me to debugging.

from gli-rs.

Oh, thank you for pointing out Drop impl for texture, I didn't notice it. Let's focus on sampler.

When I add

for _ in 0..1_000_000_000 {
  FSampler2D::new(&texture_loaded, Wrap::CLAMP_TO_EDGE, Filter::LINEAR, Filter::LINEAR);
}

before this line and run

cargo t --release

it eats 1 GB per second.

from gli-rs.

Does this line really contains shared_ptr to texture data?

If so, in Rust we should

1. add impl Drop for sampler to decrement shared_ptr counter on C++ side;
2. maybe remove lifetime type parameter for FSampler1D, FSampler2D, etc, as they contain analogue of Arc<Texture>.

from gli-rs.

I have reproduced the memory consumption when testing with FSampler2D. The memory is leaked when dropping FSampler2D, but that not reproduced on GliImage.

For the advice you recommended,

1. It seems that I neglect the inner texture member in sampler class when implementing this crate. I am trying to apply a quick fix to FSampler2D.
2. The lifetime type parameter in FSampler* structs are to remind user that the data in Texture* are shared to FSampler*. And in this way, the rust compiler also makes sure that the FSampler* is dropped before Texture*, which corresponds to the ordinary use case. Although it may not cause problem without the lifetime, the lifetime parameter does introduce the semantic relationship between Texture* and FSampler*. I want the FSampler* to use in local, instead of storing it in struct. Construct a FSampler* struct would not introduce much overhead, since it would not copy the texture data.

from gli-rs.

The binding.rs must be rebuilt. However I encounter some link errors to the C++ compiler(as both the rust and clang compiler versions are upgraded on my computer), it may take some time to fix it.

from gli-rs.

Ok, I apply a quick fix special for FSampler2D. You can try it if you are in urgent need:

# In Cargo.toml
[dependencies]
gli = { git = "https://github.com/unknownue/gli-rs", branch = "memory-fix" }

This fix is implemented in somewhat ugly way.
It should take me a few days to fix remaining parts and eliminate the warning toggled by newer version of rustc.

from gli-rs.