strings breaking coming back from unmanaged code C++

Question

I'm creating a wrapper for an unmanaged C++ function to be called using C#.
The function returns a vector of structs.
When returning from the function, the strings are all ok, but after the 'return' to the wrapper, the strings break returning weird characters. The weird thing is, if I call it a second time, without closing Visual Studio, it works!
I have saved all the files as Unicode UTF-8, set Unicode as the character set on the Visual Studio project, defined UNICODE and _UNICODE, but still the problem persists.
This is the unmanaged struct:

typedef struct SessionEnumOutput {
    SessionEnumOutput() {};
    wchar_t         *UserName;
    wchar_t         *SessionName;
    WtsSessionState SessionState;
}SessionEnumOutput, *PSessionEnumOutput;

It being built on the unmanaged function:

for (DWORD i = 0; i < pCount; i++)
    {
        WTS_SESSION_INFO_1 innerSes = sessionInfo[i];
        if (innerSes.State == WTSActive)
        {
            wchar_t *sessionName;
            wchar_t *sessUserName;
            SessionEnumOutput outObj;

            if (innerSes.pUserName == NULL) { sessUserName = L"System"; }
            else { sessUserName = innerSes.pUserName; }
            if (innerSes.pSessionName == NULL) { sessionName = L""; }
            else { sessionName = innerSes.pSessionName; }

            Unmanaged::SessionEnumOutput inner;
            inner.UserName = sessUserName;
            inner.SessionName = sessionName;
            inner.SessionState = (WtsSessionState)innerSes.State;

            output.push_back(inner);
        }
    }

The managed wrapper:

ref class wSessionEnumOutput
    {
    public:
        String^ UserName;
        String^ SessionName;
        wWtsSessionState SessionState;
    };

    List<wSessionEnumOutput^>^ GetEnumeratedSession(String^ computerName, bool onlyActive, bool excludeSystemSessions)
    {
        pin_ptr<const wchar_t> wName = PtrToStringChars(computerName);
        List<wSessionEnumOutput^>^ output = gcnew List<wSessionEnumOutput^>();
        vector<Unmanaged::SessionEnumOutput> *result = new vector<Unmanaged::SessionEnumOutput>;
        *result = ptr->GetEnumeratedSession((LPWSTR)wName, onlyActive, excludeSystemSessions);
        
        for (size_t it = 0; it < result->size(); it++)
        {
            Unmanaged::SessionEnumOutput single = result->at(it);
            wSessionEnumOutput^ inner = gcnew wSessionEnumOutput();
            inner->UserName = Marshal::PtrToStringUni((IntPtr)single.UserName);
            inner->SessionName = Marshal::PtrToStringUni((IntPtr)single.SessionName);
            inner->SessionState = (wWtsSessionState)single.SessionState;
            output->Add(inner);
        }

I can see the strings broken at *vectorUnmanaged::SessionEnumOutput result = new vectorUnmanaged::SessionEnumOutput;
I have created a test console to call the C# function twice to analyze the heap:

List<Managed.wSessionEnumOutput> thing = Utilities.GetComputerSession();
        Console.WriteLine("###################################################");
        for (int i = 0; i < thing.Count; i++)
        {
            Console.WriteLine("User name: " + thing[i].UserName);
            Console.WriteLine("Session name: " + thing[i].SessionName);
            Console.WriteLine("Session state: " + thing[i].SessionState);
            Console.WriteLine("###################################################");
        }
        Console.WriteLine("\n");
        thing = Utilities.GetComputerSession();
        Console.WriteLine("###################################################");
        for (int i = 0; i < thing.Count; i++)
        {
            Console.WriteLine("User name: " + thing[i].UserName);
            Console.WriteLine("Session name: " + thing[i].SessionName);
            Console.WriteLine("Session state: " + thing[i].SessionState);
            Console.WriteLine("###################################################");
        }

The difference is, on the second call, I can see Unicode and UTF-8 decoders loaded on the heap.
On the first call, they are not there.

here are both call's results:

I'm not a developer, just a curious system administrator, so pardon my coding habilities.
What am I missing?

EDIT:
Function definition:

    vector<Unmanaged::SessionEnumOutput> Unmanaged::GetEnumeratedSession(
    LPWSTR computerName = NULL,
    BOOL onlyActive = 0,
    BOOL excludeSystemSessions = 0
)
{
    HANDLE session;
    BOOL enumResult;
    DWORD pCount = 0;
    DWORD pLevel = 1;
    vector<SessionEnumOutput> output;

    PWTS_SESSION_INFO_1 sessionInfo = (PWTS_SESSION_INFO_1)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(WTS_SESSION_INFO_1));

    if (computerName != NULL)
    {
        session = WTSOpenServer(computerName);
        if (session == NULL) { goto END; }
    }
    else { session = WTS_CURRENT_SERVER_HANDLE; }

    enumResult = WTSEnumerateSessionsEx(session, &pLevel, 0, &sessionInfo, &pCount);
    if (enumResult == 0) { goto END; }

    switch (onlyActive)
    {
    case 1:
        for (DWORD i = 0; i < pCount; i++)
        {
            WTS_SESSION_INFO_1 innerSes = sessionInfo[i];
            if (innerSes.State == WTSActive)
            {
                wchar_t *sessionName;
                wchar_t *sessUserName;
                SessionEnumOutput outObj;

                if (innerSes.pUserName == NULL) { sessUserName = L"System"; }
                else { sessUserName = innerSes.pUserName; }
                if (innerSes.pSessionName == NULL) { sessionName = L""; }
                else { sessionName = innerSes.pSessionName; }

                Unmanaged::SessionEnumOutput inner;
                inner.UserName = sessUserName;
                inner.SessionName = sessionName;
                inner.SessionState = (WtsSessionState)innerSes.State;

                output.push_back(inner);
            }
        }
        break;

    default:
        if (excludeSystemSessions == 0)
        {
            for (DWORD i = 0; i < pCount; i++)
            {
                WTS_SESSION_INFO_1 innerSes = sessionInfo[i];
                wchar_t* sessionName;
                wchar_t* sessUserName;
                SessionEnumOutput outObj;

                if (innerSes.pUserName == NULL) { sessUserName = L"System"; }
                else { sessUserName = innerSes.pUserName; }
                if (innerSes.pSessionName == NULL) { sessionName = L""; }
                else { sessionName = innerSes.pSessionName; }

                Unmanaged::SessionEnumOutput inner;
                inner.UserName = sessUserName;
                inner.SessionName = sessionName;
                inner.SessionState = (WtsSessionState)innerSes.State;

                output.push_back(inner);

            }
        }
        else
        {
            for (DWORD i = 0; i < pCount; i++)
            {
                WTS_SESSION_INFO_1 innerSes = sessionInfo[i];
                wstring sessUserName;
                if (innerSes.pUserName == NULL) { sessUserName = L""; }
                else { sessUserName = innerSes.pUserName; }

                if (sessUserName.length() > 0)
                {
                    wchar_t *innerUser = (wchar_t*)sessUserName.c_str();
                    wchar_t *sessionName;
                    SessionEnumOutput outObj;
                    WTS_SESSION_INFO_1 innerSes = sessionInfo[i];

                    if (innerSes.pSessionName == NULL) { sessionName = L""; }
                    else { sessionName = innerSes.pSessionName; }

                    Unmanaged::SessionEnumOutput inner;
                    inner.UserName = innerUser;
                    inner.SessionName = sessionName;
                    inner.SessionState = (WtsSessionState)innerSes.State;

                    output.push_back(inner);
                }
            }
        }
        break;
    }



END:
    if (session != NULL) { WTSCloseServer(session); }
    if (pCount > 0) { WTSFreeMemoryEx(WTSTypeSessionInfoLevel1, sessionInfo, pCount); }
    return output;
}

Unclear who is supposed to create and destroy these strings, also why you don't just use normal PInvoke marshalling rather than messing with a C++/CLI wrapper. Can you show the unmanaged function's definition? — Charlieface, Aug 12 '22 at 12:53
I want to create them. they come from the WTSEnumerateSessions function. I'm not using Pinvoke cause this way gives me more flexibility. I edited the question to add the function. Thanks! — FranciscoNabas, Aug 12 '22 at 13:17
The strings in WTS_SESSION_INFO_1 become dangling pointers after the WTSFreeMemoryEx() call. You must copy the string content, not just the string pointer, to avoid the memory corruption that causes. — Hans Passant, Aug 12 '22 at 13:51
This all boils down to C++ scoping rules and pointers. It has nothing to do with C#. The same code would have failed if the entire app was written in C++. — PaulMcKenzie, Aug 12 '22 at 14:44

score 0 · Accepted Answer · answered Aug 12 '22 at 15:01

Solved the mystery!
Mr. CharlieFace on the comments posted another question where they are discussing the unespected behavior of the function WTSEnumerateSessionsEx.
Unfortunately, this is an issue happening on Windows for some time now.

I've followed the approach of calling WTSEnumerateSessions and then WTSQuerySessionInformation to get the user name.

Fragment:

PWTS_SESSION_INFO sessionInfo = (PWTS_SESSION_INFO)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(WTS_SESSION_INFO));

if (computerName != NULL)
{
    session = WTSOpenServer(computerName);
    if (session == NULL) { goto END; }
}
else { session = WTS_CURRENT_SERVER_HANDLE; }

enumResult = WTSEnumerateSessions(session, 0, 1, &sessionInfo, &pCount);
if (enumResult == 0) { goto END; }

switch (onlyActive)
{
case 1:
    for (DWORD i = 0; i < pCount; i++)
    {
        WTS_SESSION_INFO innerSes = sessionInfo[i];


        if (innerSes.State == WTSActive)
        {
            wchar_t *sessionName;
            wchar_t *sessUserName;
            LPWSTR ppBuffer;
            DWORD pBytesReturned;
            BOOL thisResult;

            thisResult = WTSQuerySessionInformation(session, innerSes.SessionId, WTSUserName, &ppBuffer, &pBytesReturned);
            if (thisResult == 0) { goto END; }
            if (ppBuffer == NULL) { sessUserName = L"System"; }
            else { sessUserName = ppBuffer; }

            if (innerSes.pWinStationName == NULL) { sessionName = L""; }
            else { sessionName = innerSes.pWinStationName; }

            Unmanaged::SessionEnumOutput inner;
            inner.UserName = sessUserName;
            inner.SessionName = sessionName;
            inner.SessionState = (WtsSessionState)innerSes.State;

            output.push_back(inner);
            WTSFreeMemory(&ppBuffer);
        }
    }
    break;

Look how pretty that is!

Sources:

Why does WTSFreeMemoryExA always return ERROR_INVALID_PARAMETER when passed a WTSTypeClass of WTSTypeSessionInfoLevel1?
Memory leak issues with Windows API call - Delphi

Thank you very much for the help!

`inner.UserName = sessUserName; inner.SessionName = sessionName;` -- You are only lucky this "worked". The issue still persists that you are using a dangling pointer in the C++ code. Go to a different C++ compiler, or use different compile options, and don't be shocked if the output won't be correct. — PaulMcKenzie, Aug 12 '22 at 17:26
In reality, your code is not valid C++, as you cannot assign a constant string-literal to a non-const character pointer `wchar_t* sessUserName;...sessUserName = L"System";` -- this is not legal C++. That variable either has to be `const wchar_t*`, but that now opens up more issues, since you are using a non-const `ppBuffer` to assign to a const pointer. Heads I win, tails you lose. So in reality, you do have to allocate memory and copy the string -- right now you are getting away with invalid C++ that just happens to "work". — PaulMcKenzie, Aug 12 '22 at 20:59
Maybe initialize the pointer in `innerSes` as `nullptr`, then the C++ code doesn't have to deal with any string constants, and just assign `ppBuffer` directly to `innerSes`, regardless of the value of `ppBuffer`. On the C# side check for `nullptr` and display the appropriate string. — PaulMcKenzie, Aug 12 '22 at 21:14

Charlieface · Answer 2 · 2022-08-12T17:02:30.120

As I mentioned, there is an outstanding bug involving the WTS functions when used with the ANSI versions. Instead you should use the Unicode versions.

See Why does WTSFreeMemoryExA always return ERROR_INVALID_PARAMETER when passed a WTSTypeClass of WTSTypeSessionInfoLevel1?

I think using a C++/CLI wrapper for this to be able to use in C# is overkill. You should be able to do this using standard PInvoke marshalling in C#.

It's best not to rely on BestFitMapping, and instead specify the function names explicitly.

[DllImport("Wtsapi32.dll", CharSet = CharSet.Unicode, BestFitMapping = false, SetLastError = true)]
static extern IntPtr WTSOpenServerExW (string pServerName);

[DllImport("Wtsapi32.dll", CharSet = CharSet.Unicode, BestFitMapping = false, SetLastError = true)]
static extern void WTSCloseServer(IntPtr hServer);

[DllImport("Wtsapi32.dll", CharSet = CharSet.Unicode, BestFitMapping = false, SetLastError = true)]
static extern bool WTSEnumerateSessionsExW(
  IntPtr hServer,
  ref int pLevel,
  int Filter,
  out IntPtr ppSessionInfo,
  out int pCount
);

[DllImport("Wtsapi32.dll", CharSet = CharSet.Unicode, BestFitMapping = false, SetLastError = true)]
static extern bool WTSFreeMemoryExW(
  WTS_TYPE_CLASS WTSTypeClass,
  IntPtr pMemory,
  int NumberOfEntries
);

enum WTS_TYPE_CLASS
{
    WTSTypeProcessInfoLevel0,
    WTSTypeProcessInfoLevel1,
    WTSTypeSessionInfoLevel1
}

enum WtsSessionState
{
    WTSActive,
    WTSConnected,
    WTSConnectQuery,
    WTSShadow,
    WTSDisconnected,
    WTSIdle,
    WTSListen,
    WTSReset,
    WTSDown,
    WTSInit
}

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
struct WTS_SESSION_INFO
{
    public int ExecEnvId;
    public WtsSessionState State;
    public int SessionId;
    [MarshalAs(UnmanagedType.LPTStr)]
    public string pSessionName;
    [MarshalAs(UnmanagedType.LPTStr)]
    public string pHostName;
    [MarshalAs(UnmanagedType.LPTStr)]
    public string pUserName;
    [MarshalAs(UnmanagedType.LPTStr)]
    public string pDomainName;
    [MarshalAs(UnmanagedType.LPTStr)]
    public string pFarmName;
}

List<SessionEnumOutput> GetEnumeratedSession(
    string computerName = null,
    bool onlyActive = false,
    bool excludeSystemSessions = false
)
{
    IntPtr server = default;
    IntPtr sessionInfo = default;
    int pCount = default;
    List<SessionEnumOutput> output = new List<SessionEnumOutput>();

    if (computerName != null)
    {
        server = WTSOpenServerExW(computerName);
        if (server == IntPtr.Zero || server == new IntPtr(-1))
            throw new Exception("Invalid computer name");
    }

    try
    {
        int pLevel = 1;
        if (!WTSEnumerateSessionsExW(server, ref pLevel, 0, out sessionInfo, out pCount))
            throw new Win32Exception(Marshal.GetLastWin32Error());

        for (var i = 0; i < pCount; i++)
        {
            WTS_SESSION_INFO innerSes = Marshal.PtrToStructure<WTS_SESSION_INFO>(sessionInfo + i * Marshal.SizeOf<WTS_SESSION_INFO>());

            if (onlyActive && innerSes.State != WtsSessionState.WTSActive
                || excludeSystemSessions && innerSes.pSessionName == null)
                continue;

            SessionEnumOutput inner = new SessionEnumOutput
            {
                UserName = innerSes.pUserName ?? "System",
                SessionName = innerSes.pSessionName ?? "",
                SessionState = innerSes.State,
            };

            output.Add(inner);
        };
    }

    finally
    {
        if (sessionInfo != default)
            WTSFreeMemoryExW(WTS_TYPE_CLASS.WTSTypeSessionInfoLevel1, sessionInfo, pCount);
        if (server != default)
            WTSCloseServer(server);
    }

    return output;
}

Note the use of a finally to enure memory is freed correctly.

There is also a huge amount of duplicated code. I have cleaned up the rest of the function significantly.

Yep, using C++/CLI was more an adventure than for functionality. I'll start using the function names instead of the mappings. Thanks for the help! — FranciscoNabas, Aug 12 '22 at 16:52

strings breaking coming back from unmanaged code C++

2 Answers2