Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

C# SafeHandles purpose? Why Steamworks .Net using it in this situation (UTF8StringHandle)?

Discussion in 'Scripting' started by HiddenMonk, Jul 2, 2018.

  1. HiddenMonk

    HiddenMonk

    Joined:
    Dec 19, 2014
    Posts:
    987
    I have been looking at the Steamworks .Net source code to see what might cause garbage and have noticed methods like SetLobbyData and GetLobbyData look like this..
    Code (CSharp):
    1. public static bool SetLobbyData(CSteamID steamIDLobby, string pchKey, string pchValue) {
    2.             InteropHelp.TestIfAvailableClient();
    3.             using (var pchKey2 = new InteropHelp.UTF8StringHandle(pchKey))
    4.             using (var pchValue2 = new InteropHelp.UTF8StringHandle(pchValue)) {
    5.                 return NativeMethods.ISteamMatchmaking_SetLobbyData(CSteamAPIContext.GetSteamMatchmaking(), steamIDLobby, pchKey2, pchValue2);
    6.             }
    7.         }
    Code (CSharp):
    1. [DllImport(NativeLibraryName, EntryPoint = "SteamAPI_ISteamMatchmaking_SetLobbyData", CallingConvention = CallingConvention.Cdecl)]
    2.         [return: MarshalAs(UnmanagedType.I1)]
    3.         public static extern bool ISteamMatchmaking_SetLobbyData(IntPtr instancePtr, CSteamID steamIDLobby, InteropHelp.UTF8StringHandle pchKey, InteropHelp.UTF8StringHandle pchValue);

    This is what InteropHelp.UTF8StringHandle is...
    Code (CSharp):
    1. // This is for 'const char *' arguments which we need to ensure do not get GC'd while Steam is using them.
    2.         // We can't use an ICustomMarshaler because Unity crashes when a string between 96 and 127 characters long is defined/initialized at the top of class scope...
    3. #if UNITY_EDITOR || UNITY_STANDALONE || STEAMWORKS_WIN || STEAMWORKS_LIN_OSX
    4.         public class UTF8StringHandle : Microsoft.Win32.SafeHandles.SafeHandleZeroOrMinusOneIsInvalid {
    5.             public UTF8StringHandle(string str)
    6.                 : base(true) {
    7.                 if (str == null) {
    8.                     SetHandle(IntPtr.Zero);
    9.                     return;
    10.                 }
    11.  
    12.                 // +1 for '\0'
    13.                 byte[] strbuf = new byte[Encoding.UTF8.GetByteCount(str) + 1];
    14.                 Encoding.UTF8.GetBytes(str, 0, str.Length, strbuf, 0);
    15.                 IntPtr buffer = Marshal.AllocHGlobal(strbuf.Length);
    16.                 Marshal.Copy(strbuf, 0, buffer, strbuf.Length);
    17.  
    18.                 SetHandle(buffer);
    19.             }
    20.  
    21.             protected override bool ReleaseHandle() {
    22.                 if (!IsInvalid) {
    23.                     Marshal.FreeHGlobal(handle);
    24.                 }
    25.                 return true;
    26.             }
    27.         }
    28. #else
    29.         public class UTF8StringHandle : IDisposable {
    30.         public UTF8StringHandle(string str) { }
    31.         public void Dispose() {}
    32.         }
    33. #endif
    34.  

    Notice the note "This is for 'const char *' arguments which we need to ensure do not get GC'd while Steam is using them."
    So to my understanding, this SafeHandle class will basically prevent the object from being destroyed as well as lock the pointer so that the garbage collection doesnt move it, this way the unmanaged code wont lose its pointer to it.
    However, if thats the case, then I dont really see why its needed in the SetLobbyData method.
    Is it because the pointer might change during the SetLobbyData call even before it finished returning? Is that a thing? Maybe due to multithreading?
    If we were to just pass a string, I thought the pinvoke marshalling would behind the scenes create a temporary copy of the string and convert it to a const char * which would only be destroyed after the pinvoke returns. Wouldnt that give the same result as with the current use of UTF8StringHandle?

    I went and looked at FacePunch Steamworks implementation and saw that they seem to just pass the string normally.
    Here is how they do it...
    Code (CSharp):
    1. public virtual bool /*bool*/ ISteamMatchmaking_SetLobbyData( ulong steamIDLobby, string /*const char **/ pchKey, string /*const char **/ pchValue )
    2.             {
    3.                 if ( _ptr == IntPtr.Zero ) throw new System.Exception( "ISteamMatchmaking _ptr is null!" );
    4.        
    5.                 return Native.SteamAPI_ISteamMatchmaking_SetLobbyData(_ptr, steamIDLobby, pchKey, pchValue);
    6.             }
    Code (CSharp):
    1. [DllImport( "steam_api64.dll" )] internal static extern bool /*bool*/ SteamAPI_ISteamMatchmaking_SetLobbyData( IntPtr ISteamMatchmaking, ulong steamIDLobby, string /*const char **/ pchKey, string /*const char **/ pchValue );
    Are they doing it right, or is Steamworks .Net doing it right and FacePunch version is at risk of failing?

    I dont want to use the UTF8StringHandle since it seems to create a byte array which would generate garbage. Its bad enough I need to deal with all the strings these methods return... (working on a way to just grab and work with the bytes).
    Since I might be calling SetLobbyData and GetLobbyData frequently, I am trying to stray from all the allocations.

    Edit -
    Actually I think the Marshal.AllocHGlobal within the UTF8StringHandle is what locks and keeps the object alive, I guess the SafeHandle class itself is just to help clean things up.
    Also, I think the Garbage collection runs on a separate thread and could run even in the middle of the pinvoke call.
    However, I still dont see why the UTF8StringHandle is needed in SetLobbyData. From what I read, when passing a string to a pinvoke method it will automatically either copy and pin it or just pin the original until the pinvoke returns.
    Also, when doing a test in unity with just sending the string, the profiler shows no garbage being generated, so I guess if it is temporarily creating a string, its doing so in a way that doesnt bother the garbage collector. However, when using the UTF8StringHandle, its causing lots of garbage. Caching the UTF8StringHandle and reusing it causes no garbage, as expected, but it would be locking that string in the heap, causing it to make the garbage collector work harder to not touch it.

    Edit2-
    After researching some more and learning about this stuff, I think the main purpose of the UTF8StringHandle is to just be a way to pass UTF8 strings to the pinvoke. It seems that the default encoding used by the auto marshalling is ascii or ansi and there is no current way to make it convert to utf8 so you would need to do it yourself similar to how its done in the UTF8StringHandle class.
    So while you could do it how facepunch steamworks does it by just passing the string, if you want utf8 encoding you will need to handle it yourself like in steamworks .net.

    Now the question is, is there a less wasteful way of doing this? If the auto marshaller seems to do it without generating garbage, I wonder if we can too.
     
    Last edited: Jul 3, 2018