gui-cs/NStack

IntPtrUString liveness issues

Therzok opened this issue · 6 comments

  1. IDisposable implementation is hidden to ustring API users, thus IntPtrUString will always end up being finalized, unless people explicitly Dispose the string.

  2. IntPtrUString design assumes that the parent string outlives the child string, failing code would look like:

IntPtr mem = grab_native_string(out int count);
var parent = ustring.Make (mem, count, str => release_native_string (str));
var child = parent [0, 5]; // Let's assume these values are correct
// somewhere here GC happens, parent is reclaimed
child.ToString(); // accessing bad memory

Maybe a SafeHandle mechanism is better here than an opaque pointer? The wrappers can get reclaimed while the SafeHandle still lives, so that's 👍.

That would make the IntPtrUString slightly larger - Marshal.SizeOf(SafeHandle) + sizeof(int) for an offset.

And by the fact that the strings will end up being finalized means that all strings get promoted to gen1, thus taking much longer for them to be reclaimed. In a performance-tailored string processing environment this can be dreadful.

Give or take, the safehandle approach is the only decent one I can come up with that makes the GC aware of the memory release semantics. We could get rid of IDisposable completely, so we don't have to deal with user error - Dispose the string then get a subrange and crash.

With this approach we only queue for finalization once per safehandle instead of once per string, thus, a net win.

Mhm, I can optimize the case of ustring pointing to a memory block, that has no release func, at least those would not be promoted to gen1 like you point out. But that would still leave those that require finalizers to take that hit.

The discoverability of IDisposable is not obvious, but I use it in the code/tests. Perhaps I should just make ustring IDisposable, without the finalizer and only add the finalizer to the IntPtrUString. At least that would make the IDisposable obvious.

I do not understand how SafeHandle prevents the scenario where the original containing string is released.

The safehandle reference is passed and shared between all instances. It's an object, so it won't get finalized until all referencers are claimable. Moving the ownership of the ptr to the safehandle fixes that issue.

#13 should address this.

Checked in a variation that does not require the SafeHandle.