FacticiusVir / SharpVk

C# Bindings for the Vulkan API & SPIR-V
MIT License
147 stars 18 forks source link

AddrOfPinnedObject on not pinned object. #34

Open Kaktusbot opened 7 years ago

Kaktusbot commented 7 years ago

https://github.com/FacticiusVir/SharpVk/blob/master/SharpVk/SharpVk/CommandBuffer.cs#L490-L492

var arrayValue = offsets.GetArrayValue();
offsetsHandle = GCHandle.Alloc(arrayValue.Array);
marshalledOffsets = (DeviceSize*)(offsetsHandle.AddrOfPinnedObject() + (int)(MemUtil.SizeOf<DeviceSize>() * arrayValue.Offset)).ToPointer();

Maybe I don't understand something, but here not pinned GCHandle calls AddOfPinnedObject() and I have exception throwing on that line. With this its working fine GCHandle.Alloc(arrayValue.Array, GCHandleType.Pinned);

FacticiusVir commented 7 years ago

Interesting; GCHandleType.Pinned is the correct usage - as per the reference in MemUtil - must just be a fluke that I've not seen that exception in testing. What OS/Framework are you testing on?

Kaktusbot commented 7 years ago

I'm on windows with .NET 4.6.1 AddrOfPinnedObject is throwing error. Msdn says it throws that error if GCHandleType is other type than Pinned, and intellisense says that single parameter overload GCHandle.Alloc(arrayValue.Array) return GCHandle of type Normal.

And, I do not want to open second issue in row, but I probably found unmanaged memory leak in samples. Without calling commandPool.FreeCommandBuffers(commandBuffers); in RecreateSwapChain which is not called in samples I get memory leaks on resizing window. I don't know proper way of finding unmanaged leaks, but if I call GC.Collect() on each frame without freeing command buffers and non-stop resizing window I see constantly increasing memory usage in windows task manager.

FacticiusVir commented 7 years ago

I'll need to update the generator to fix it, and some of the new command names in patch 42 have broken the parser so I'll get those sorted and uploaded. The memory leak is down to the buffers not being released to the pool, will raise an issue on the Samples project.

FacticiusVir commented 7 years ago

Should be resolved in the latest commit.

Kaktusbot commented 7 years ago

Yeah. Do you mind if I close issue only after generator update?

FacticiusVir commented 7 years ago

Sure - I'm working on a large rewrite of the generator to improve the code structure and maintainability so it'll be a while before that's done, but in the meantime the manual fix should unblock anyone affected by this bug.