y-crdt / ydotnet

.NET bindings for yrs.
MIT License
35 stars 9 forks source link

Investigate possible memory leaks #34

Open LSViana opened 11 months ago

LSViana commented 11 months ago

In #18, there has been a mention of possible memory leaks in the operations executed through YDotNet.

This issue intends to:

  1. Investigate the possible issues
  2. Document the discoveries
  3. Create a procedure and, possibly, an automated way to detect such issues in the future

Investigated scenarios (this list is not final):


Other fixes:

LSViana commented 11 months ago

First round of investigations:

  1. Creating documents without explicitly disposing them leaves the memory disposal for the GC, which can cause a very high memory usage until the operating system memory is close to exhaustion (this behavior was seen in both Windows and macOS) but it's still freed as expected
  2. Creating documents and explicitly disposing them has constant memory usage

For instance, this screenshot demonstrates the scenario (2):

image

Further investigations will be conducted for other operations.

LSViana commented 11 months ago

Second round of investigations:

  1. It was discovered that creating Doc instances with DocOptions that included string properties would cause an increasing usage of memory until the Dispose() method of DocOptionsNative was called
  2. This was improved to call that Dispose() method manually and now the memory usage to create Doc instances with DocOptions is constant too
LSViana commented 11 months ago

Third round of investigations:

  1. To prevent relying on clients of the library to invoke Dispose() on resources, I'm implementing destructors on public classes exposed by the library.
  2. The private or internal classes or structs shouldn't implement Dispose() since it's expected that the library, internally, calls Dispose() correctly wherever it's needed.
SebastianStehle commented 11 months ago

I think input will also leak memory. e.g. when creating a new collection input a new block of memory is allocated and just the pointer is given to y-crdt. But the pointer is never released. I think a finalizer is needed here.

SebastianStehle commented 2 months ago

@LSViana Can we close this?

LSViana commented 2 months ago

@SebastianStehle No, the initial work I did here was useful for detecting and fixing memory leaks.

Given the previous discoveries, I believe it's important to check for possible leaks in the pending items too. It's not a complex task, it's more laborious and repetitive, I'll try to do a little bit every day.