A bit of a weird dispose pattern - Concerns on leaky resources?

[TomTheFurry]

I have been getting an (unrelated) issue with glsl include sub-includes sometimes seemingly (probably?) accessing freed file data. So, when I am debugging that, I noticed in here, if the Result object isn't explicitly disposed, (i.e. instead, letting c# GC handle it), the ~Result() finalizer actually doesn't do anything? Cause the native Result object is apparently only freed if you call the explicit Dispose()?

I have also seen this appears in many other places, with (I think) native resources being conditionally freed behind that bool flag. So I am really not sure if this is a bug or if I am missing something?

Code:

Vortice.Vulkan/src/Vortice.ShaderCompiler/Result.cs

Lines 37 to 57 in 984b679

	/// <summary>
	/// Finalizes an instance of the <see cref="Result" /> class.
	/// </summary>
	~Result() => Dispose(disposing: false);
	public void Dispose()
	{
	Dispose(true);
	GC.SuppressFinalize(this);
	}
	protected virtual void Dispose(bool disposing)
	{
	if (_handle == 0)
	return;
	if (disposing)
	{
	shaderc_result_release(_handle);
	}
	}

[amerkoleci]

Can you try to follow the IDisposable pattern using keyword and see if it causes issue?

Take look here:
https://github.com/amerkoleci/Vortice.Vulkan/blob/main/tests/Vortice.ShaderCompiler.Test/CompileTests.cs#L19

[amerkoleci]

Share some code so I can gladly help

[TomTheFurry]

Trying to zoom in the scope atm, and extracting a standalone repro case. (I was doing some company code, so, yeah.)

[TomTheFurry]

And... how the heck did a #line directive even happen!? I swear nowhere in any shader file contains any #line directives!

Plz don't tell me it's a deeper shaderc issue...

[TomTheFurry]

Okay yeah not sure what's going on. It just sometimes randomly fails on a #include, and retrying again and again then sometimes works. Hmm...

[amerkoleci]

Just released version https://www.nuget.org/packages/Vortice.ShaderCompiler/1.8.0 which improves all this stuff, can you test?

[TomTheFurry]

Update, can confirm that the latest (ShaderCompiler v1.8.0) did fix above mentioned issues!

Btw, it might be a good idea to add comments on Compiler class that it REQUIRES explicit call to Dispose().

Side-note:

My current guess on why the shaderc is reading corrupted data on #include is that the old version of passing included file strings into native environment seems to pass incorrect 'length' param compared to the raw utf-8 c-string it passes:

Vortice.Vulkan/src/Vortice.ShaderCompiler/Includer.cs

Lines 75 to 76 in f75763a

	result->content = Utils.ToUTF8(fileContent);
	result->content_length = (nuint)fileContent.Length;

In above, it is passing down the utf-16 c#-style string Length as the utf-8 string's 'length' parameter, meaning a mismatch of what it say the string length is vs what the actual string length is.

Now, how, or even if, those mismatches lead to the memory corruption, I have no idea. This really is just a guess. It prob was causing problems. But is that the specific problem I was encountering? Who knows.

I can confirm however that the new reworked code indeed does this properly, and that the issues on corrupted memory no longer (seems to) happen. Thanks @amerkoleci a lot!