Subtleties of .NET and a SlimDX Crash Bug

posted in Promit's Ventspace

Published October 01, 2009

Let's look at a real, in the trenches SlimDX bug that results from a problematic subtlety in .NET. This one isn't even fixed in trunk yet. Here's our main function of interest:

generic where T : value class        Result BaseEffect::SetValue( EffectHandle^ parameter, T value )        {                HRESULT hr;                D3DXHANDLE handle = parameter != nullptr ? parameter->InternalHandle : NULL;                if( T::typeid == bool::typeid )                {                        BOOL newValue = Convert::ToInt32( value, CultureInfo::InvariantCulture );                        hr = InternalPointer->SetBool( handle, newValue );                }                else if( T::typeid == float::typeid )                {                        hr = InternalPointer->SetFloat( handle, static_cast<FLOAT>( value ) );                }                else if( T::typeid == int::typeid )                {                        hr = InternalPointer->SetInt( handle, static_cast<INT>( value ) );                }                else if( T::typeid == Matrix::typeid )                {                        hr = InternalPointer->SetMatrix( handle, reinterpret_cast( &value ) );                }                else if( T::typeid == Vector4::typeid )                {                        hr = InternalPointer->SetVector( handle, reinterpret_cast( &value ) );                }                else                {                        hr = InternalPointer->SetValue( handle, &value, static_cast( sizeof(T) ) );                }                return RECORD_D3D9( hr );        }

This function will crash randomly on good input. Specifically, D3D will return a D3DERR_INVALIDCALL, which then gets translated to an exception. Again, that's on known-good input. The call to the function looks like this:

effect.SetValue("alpha", 0.5f);

When this call is made, an EffectHandle is implicitly constructed that copies the string into native memory. Here's the relevant bits of EffectHandle:

        EffectHandle::EffectHandle( String^ name )        {                m_StringData = Marshal::StringToHGlobalAnsi( name );                m_HasString = true;                m_StringDataSize = name->Length;                GC::AddMemoryPressure( m_StringDataSize );                m_Handle = reinterpret_cast( m_StringData.ToPointer() );        }//Called by both ~EffectHandle and !EffectHandle.        void EffectHandle::Destruct()        {                if( m_HasString )                {                        Marshal::FreeHGlobal( m_StringData );                        GC::RemoveMemoryPressure( m_StringDataSize );                }        }

So. Do you see where we screwed this up?

In case it's not clear:
* EffectHandle::InternalHandle returns EffectHandle::m_Handle
* D3DXHANDLE is a typedef for char*
* Although there's plenty of interop, the interop is NOT the root cause.

HINT: EffectHandle is finalizable. Answer is in comments.

Previous Entry Oh hell yes.

Next Entry Checklist to Lead an Online Game Project

0 likes 12 comments

Comments

bubu LV

hm... m_HasString is not being set to false in Destruct() method?

October 01, 2009 01:17 PM

Promit

While you could make an argument for that being a good idea, it's not why it's crashing. (Though it might have made the crash easier to find.)

October 01, 2009 01:32 PM

bubu LV

Ah, I think I got it.

Is it because after this line:

D3DXHANDLE handle = parameter != nullptr ? parameter->InternalHandle : NULL;

parameter object is no more referenced in function? So GC is free to dispose it. And when it is disposing it, then parameter->Destruct() is called and m_StringData is freed. But later in InternalPointer->SetFloat call it's memory is used (with native handle pointer).

October 01, 2009 02:22 PM

Promit

That is exactly it. You win a Rate++. Do you know how to solve it?

October 01, 2009 02:45 PM

bubu LV

:)

You could add GC::KeepAlive(parameter) before return RECORD_D3D9( hr ); line.

But first I thought you could use "parameter != nullptr ? parameter->InternalHandle : NULL" in each place where currently "handle" is written. But that is still incorrect - that would still not prevent GC from runing while current thread is inside DirectX call.

October 01, 2009 03:04 PM

Promit

Yep, correct again. Although the fix actually involves adding it in like thirty functions [sad]

October 01, 2009 03:22 PM

kirkus

Would this be a good situation for a SafeHandle?

October 02, 2009 08:50 AM

Promit

No, SafeHandle is for Win32 HANDLE, which is an entirely different beast.

October 02, 2009 10:33 AM

kirkus

Interesting. The MSDN documentation uses this as an example of a SafeHandle for unmanaged pointers:


// Demand unmanaged code permission to use this class.
    [SecurityPermission(SecurityAction.Demand, UnmanagedCode = true)]
    sealed class SafeUnmanagedMemoryHandle : SafeHandleZeroOrMinusOneIsInvalid
    {

        // Set ownsHandle to true for the default constructor.
        internal SafeUnmanagedMemoryHandle() : base(true) { }

        // Set the handle and set ownsHandle to true.
        internal SafeUnmanagedMemoryHandle(IntPtr preexistingHandle, bool ownsHandle)
            : base(ownsHandle)
        {
            SetHandle(preexistingHandle);
        }

        // Perform any specific actions to release the 
        // handle in the ReleaseHandle method.
        // Often, you need to use Pinvoke to make
        // a call into the Win32 API to release the 
        // handle. In this case, however, we can use
        // the Marshal class to release the unmanaged
        // memory.
        override protected bool ReleaseHandle()
        {
            // "handle" is the internal
            // value for the IntPtr handle.

            // If the handle was set,
            // free it. Return success.
            if (handle != IntPtr.Zero)
            {

                // Free the handle.
                Marshal.FreeHGlobal(handle);

                // Set the handle to zero.
                handle = IntPtr.Zero;

                // Return success.
                return true;

            }

            // Return false. 
            return false;

        }
    }

October 02, 2009 10:43 AM

jvsstudios

What about passing parameter in by reference? To me that would seem to keep the GC from disposing it since it would be referenced outside of the function.

October 03, 2009 09:42 AM

jvsstudios

Oops, nevermind. It looks like that is already being done. But if I were to guess, I would say pass the EffectHandle in as a const. If I am wrong don't hurt me too much :) I do program in c++, but I have not worked with managed c++.

October 03, 2009 09:54 AM

bubu LV

Afaik managed types don't have const types like native C++.

October 05, 2009 10:02 AM

You must log in to join the conversation.

Don't have a GameDev.net account? Sign up!

Promit

Author

Subtleties of .NET and a SlimDX Crash Bug

Comments

Promit

Latest Entries

Fully Featured Custom Build Targets in Visual C++

DanceForce V4 DIY DDR Pad Build Thread

Games Look Bad, Part 1: HDR and Tone Mapping

Games Look Bad, Part 0: Explanation and Defense

Game Debugging And Tweaking Via MIDI Controller

Bandit's Shark Showdown - Can Video Games Help Stroke Victims?

Sony A77 Mark II: EVF Lag and Blackout Test

Time Capsule Draft: “Speculating About Xbox Next”

Quick tip: Retina mode in iOS OpenGL rendering is not all-or-nothing

I Am Dolphin – Kinect Prototype

Subtleties of .NET and a SlimDX Crash Bug

Comments

Promit

Latest Entries

Fully Featured Custom Build Targets in Visual C++

DanceForce V4 DIY DDR Pad Build Thread

Games Look Bad, Part 1: HDR and Tone Mapping

Games Look Bad, Part 0: Explanation and Defense

Game Debugging And Tweaking Via MIDI Controller

Bandit&#39;s Shark Showdown - Can Video Games Help Stroke Victims?

Sony A77 Mark II: EVF Lag and Blackout Test

Time Capsule Draft: “Speculating About Xbox Next”

Quick tip: Retina mode in iOS OpenGL rendering is not all-or-nothing

I Am Dolphin – Kinect Prototype

Reticulating splines

Bandit's Shark Showdown - Can Video Games Help Stroke Victims?