Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory corruption after calling methods that return SList with transfer-ownership="none" #183

Closed
BwackNinja opened this issue Jan 9, 2025 · 5 comments
Labels
bug Something isn't working

Comments

@BwackNinja
Copy link

I've been trying to figure out what's causing memory corruption when I'm rendering text, and I think I've found it.

The finalizer registered to the Cleaner in the SList constructor universally calls SListNode.free(new SListNode(address)); even when java-gi doesn't own the memory and shouldn't be freeing it. Examples include Layout.getLinesReadonly() in Pango.

Retaining a reference to the SList to prevent the finalizer from running does prevent the memory corruption.

@jwharm jwharm added the bug Something isn't working label Jan 9, 2025
@jwharm
Copy link
Owner

jwharm commented Jan 9, 2025

Thanks for the bug report. These issues a really annoying to debug. This helps a lot.

@BwackNinja
Copy link
Author

Jemalloc is really nice for finding memory leaks, but finding when and what has been preemptively been free'd is tough.

This is related, so I'm not sure if this needs to be a separate report, but there are other issues also involving returns with transfer-ownership="none". Buffer.peekMemory() and Buffer.getMeta() in GStreamer run MemoryCleaner.takeOwnership(_instance); despite also being marked transfer-ownership="none".

@jwharm
Copy link
Owner

jwharm commented Jan 11, 2025

Buffer.peekMemory() calls g_boxed_copy to create its own copy or add a reference, and then takes ownership of the copy. The MemoryCleaner will call g_boxed_free to free the copy (or decrease the reference). This ensures your Java variable will not be freed suddenly, while you were still using it.

Similarly, Buffer.getMeta() creates a copy of the memory of the returned Meta struct. That memory is freed by the MemoryCleaner afterwards. (This needs more work, because the copy is "shallow": Meta contains a MetaInfo child that isn't copied.)

@BwackNinja
Copy link
Author

Ah. Thanks for explaining that.

I'm still seeing some other memory issues, at least a failure to free GdkTextures, but I'm trying to isolate that before I can confidently make a bug report. Thanks for the improvements! I'm almost back where I was with version 0.9.x without needing the explicit unref() calls.

@jwharm
Copy link
Owner

jwharm commented Jan 11, 2025

Awesome! May I ask what you are developing?

@jwharm jwharm closed this as completed in bf72e20 Jan 13, 2025
jwharm added a commit that referenced this issue Jan 13, 2025
Do not free unowned GList/GSlist (fixes #183)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants