Getting the Value from PWSTR

[JesseAtExitNorth]

I'm not familiar with Win32 APIs at all, but I'm trying to get window names for all active windows and I think my issue is in dealing with PWSTR.

        public void getAllWindowsInfo()
        { 
            Boolean windowReturn = PInvoke.EnumWindows((HWND handle, LPARAM customParam) => {

                int realWindowLength = PInvoke.GetWindowTextLength(handle);
                PWSTR windowNameChars = new PWSTR();
                //This always returns 0, and windowNameChars is empty even when GetWindowTextLength returns a value
                int windowNameLength = PInvoke.GetWindowText(handle, windowNameChars, realWindowLength);
                //This doesn't usually return zero
                Console.WriteLine("Window Name Length is "+ realWindowLength.ToString());

                String windowName = windowNameChars.ToString();
                Console.WriteLine(windowName);

                return true;
            }, 
            new LPARAM (0) );
        }

is some example code, windowNameChars is never populated even when GetWindowTextLength returns a value, so I know there is a window title. Do I need to initialize PWSTR with a char* first?

[AArnott]

You have to initialize the PWSTR to actually point to a character buffer, otherwise you effectively pass in NULL.
But if you forget the PWSTR part and just use char*, it gets a bit simpler. This works, with added error checking:

    public void GetAllWindowsInfo()
    {
        bool windowReturn = PInvoke.EnumWindows(
            (HWND handle, LPARAM customParam) =>
            {
                int bufferSize = PInvoke.GetWindowTextLength(handle) + 1;
                unsafe
                {
                    fixed (char* windowNameChars = new char[bufferSize])
                    {
                        if (PInvoke.GetWindowText(handle, windowNameChars, bufferSize) == 0)
                        {
                            int errorCode = Marshal.GetLastWin32Error();
                            if (errorCode != 0)
                            {
                                throw new Win32Exception(errorCode);
                            }

                            return true;
                        }

                        string windowName = new string(windowNameChars);
                        this.logger.WriteLine(windowName);
                    }

                    return true;
                }
            },
            (LPARAM)0);
    }
}

[JesseAtExitNorth]

Thank you, that is very helpful.

[jnm2]

If the window text changes between calls, you need to loop, right?

[Al-Lo1]

Could I also use
char* windowNameChars = stackalloc char[bufferSize];
instead of
fixed (char* windowNameChars = new char[bufferSize])
in this case?

[jnm2]

Yes, but it's considered risky to stack-allocate without a strict upper limit due to the possibility of stack overflows. That memory won't be freed until the method returns.

[netcorefan1]

Could I also use char* windowNameChars = stackalloc char[bufferSize]; instead of fixed (char* windowNameChars = new char[bufferSize]) in this case?

If your goal is to simplify code, there should be another way (look at the end of my comment). Pinning should still occur just like fixed statement, but I'm not sure if it is safe (better to wait for @AArnott confirmation).
This is how I do:

public string Caption {
    get {
           Span<char> buf = stackalloc char[GetWindowTextLength(Hwnd) + 1];
           unsafe {
                fixed (char* cBuf = buf) {
                    return buf.Slice(0, GetWindowText(Hwnd, cBuf, buf.Length)).ToString();
                }
           }
    }
}

It's a slightly modified version of a sample I found here. I'm also wondering why the fixed statement.

EDIT

This code, although could look easier and cleaner, it has some risks well explained in #181 (reply in thread). It should be avoided unless someone has no problem to write additional code to handle the two points illustrated from @jnm2. The recommended way is the sample posted from @AArnott

[jnm2]

Stack-allocating an unknown length is generally considered risky.

[Al-Lo1]

Thank you for the feedback

[netcorefan1]

Stack-allocating an unknown length is generally considered risky.

Thanks for reminding this important rule. Maximum allowed length for a Window caption is 256 chars which is rarely filled. It should be safe to go straightly on stack.

[netcorefan1]

Sorry for my delay. I didn't got any notification. What you wrote here is something that I was totally unaware of. You saved me and so many other people from such risks. I've just edited #181 (comment) with the required info to inform the users. Many thanks!!

[jnm2]

Sure! Another safeguard that is harder to overlook could be:

var textLength = GetWindowTextLength(Hwnd);
Debug.Assert(textLength <= 256);
Span<char> buf = stackalloc char[textLength + 1];

[netcorefan1]

Many thanks for this other precious tip. I never used Debug.Assert, but if I'm not wrong, the compiler will remove any debug assertion in release mode (for performance reasons). If the problems you described occur in production mode, the application will break and we will only notice the problem the next time we need to recompile. Am I correct to assume that assertion can add some layer of safety, but this is still the recommended way for full safety?

While testing different versions of the code, I also got good results in benchmarks (very near to stack allocation) with SpanOwner from the community toolkit. Things becomes even better when combined with an extension method as suggested here:

SpanOwner<T> owner = SpanOwner<T>.ShouldRent(length) ? SpanOwner<T>.Rent(length) : stackalloc T[length];
Span<T> span = owner.Span;
DoSomeWorkWithSpan(span);
owner.Dispose();

Correct me if I'm wrong, but to me this seems 100% safe, requires very minimal amount of code and still able to provide nearly the same perf.

Also, I'm still unsure on what should be the maximum stack allocation. I see people using different values and I'm wondering if it really makes differences if, in order to cover the risks you mentioned I directly stack alloc 512 bytes instead of 256.

[jnm2]

Debug.Assert could definitely be any kind of check you want. I meant it as a placeholder for an idea. It could also be if (textLength > 256) throw ... You're exactly right about Debug.Assert not detecting the issue in production. If there's no performance detriment, I do actually lean towards keeping the check in production too.

Am I correct to assume that assertion can add some layer of safety, but #181 (comment) is still the recommended way for full safety?

I'm looking at @AArnott's code that you linked there and I'm not seeing the safety- nothing prevents stack-allocating an arbitrarily high amount, unless you know that EnumWindows will only enumerate top-level windows and that Windows itself still has a 256-character limit for window title text.
Oops, I misread new as stackalloc. My apologies! Yes, it's safe, and I would tend towards using it because we don't have to think as hard about getting it right, and there's a cost to additional code complexity.

Correct me if I'm wrong, but to me this seems 100% safe, requires very minimal amount of code and still able to provide nearly the same perf.

In general, the pattern of "stack-allocate if small enough, otherwise heap-allocate or use a pool" is right on. I don't know enough about SpanOwner<T>.ShouldRent to know if it is aligned with this goal, but it could be.

Also, I'm still unsure on what should be the maximum stack allocation. I see people using different values and I'm wondering if it really makes differences if, in order to cover the risks you mentioned I directly stack alloc 512 bytes instead of 256.

There is a standard limit, but I'm not super familiar with what it usually is and whether it's in bytes or elements. Another Microsoft engineer was talking about this in https://aka.ms/dotnet-discord at https://discord.com/channels/732297728826277939/732297728826277942/1003851613565374555:

the rule of thumb is if its under 1kb, its fine; if its over you need to be careful
1024 bytes is the limit that malloca and others use on Windows (which is the function that dynamically picks stackalloc or malloc based on size)
and so you typically don't want to exceed that for any single function frame
leaf methods (that is methods which don't call any other functions) can often have a bit more leeway, but you still want to be considerate overall
modern CPUs also does special things with stack memory and caching, so that's another reason to be considerate/careful

[tannergooding]

Yep, 1kb of space (so 512 char in C#) is generally a good guideline, keeping in mind that one of those needs to be the terminating null.

[netcorefan1]

Debug.Assert could definitely be any kind of check you want. I meant it as a placeholder for an idea. It could also be if (textLength > 256) throw ... You're exactly right about Debug.Assert not detecting the issue in production. If there's no performance detriment, I do actually lean towards keeping the check in production too.

From what is stated here, when compiled in Release mode, all assertion statements are removed unless some optimizations are disabled and this will inevitably affect performance. I really need to take the habit to use assertions when playing with Debug builds. I always believed that I had to manually remove them for the release build, but given that the compiler will do everything for me, I have no excuses. Do you have some library to recommend? The one from System.Diagnostics is pretty much limited.

Am I correct to assume that assertion can add some layer of safety, but #181 (comment) is still the recommended way for full safety?

I'm looking at @AArnott's code that you linked there and I'm not seeing the safety- nothing prevents stack-allocating an arbitrarily high amount, unless you know that EnumWindows will only enumerate top-level windows and that Windows itself still has a 256-character limit for window title text.

Ohh... You mean that fixed (char* windowNameChars = new char[bufferSize]) is an implicit stack allocation even without an explicit statement?

Correct me if I'm wrong, but to me this seems 100% safe, requires very minimal amount of code and still able to provide nearly the same perf.

In general, the pattern of "stack-allocate if small enough, otherwise heap-allocate or use a pool" is right on. I don't know enough about SpanOwner<T>.ShouldRent to know if it is aligned with this goal, but it could be.

Yes, I would say this is the main goal of SpanOwner (unlike regular memory pool, the returned buffer is exactly the requested size).
I don't know what is the average rate of memory usually required to get text from all Windows (and non windows). Given your deep knowledge of the Win32 api, do you feel to recommend a particular buffer size that fits well between the two boundaries (memory pool - stack) ?

Also, I'm still unsure on what should be the maximum stack allocation. I see people using different values and I'm wondering if it really makes differences if, in order to cover the risks you mentioned I directly stack alloc 512 bytes instead of 256.

There is a standard limit, but I'm not super familiar with what it usually is and whether it's in bytes or elements. Another Microsoft engineer was talking about this in https://aka.ms/dotnet-discord at https://discord.com/channels/732297728826277939/732297728826277942/1003851613565374555:

the rule of thumb is if its under 1kb, its fine; if its over you need to be careful
1024 bytes is the limit that malloca and others use on Windows (which is the function that dynamically picks stackalloc or malloc based on size)
and so you typically don't want to exceed that for any single function frame
leaf methods (that is methods which don't call any other functions) can often have a bit more leeway, but you still want to be considerate overall
modern CPUs also does special things with stack memory and caching, so that's another reason to be considerate/careful

@tannergooding Yep, 1kb of space (so 512 char in C#) is generally a good guideline, keeping in mind that one of those needs to be the terminating null.

Thanks guys. So, If I'm not wrong this should be the right approach:
if you're wasting a minimal amount of stack memory, go ahead with stack. If you're going to do 99 calls where you expect less then 256 kb on each one, don't stack allocate 1 kb just because the hundredth call may need 900Kb (and just because is easy like changing a number with no additional handling code). It will be a waste of memory and you're better off with a memory pool.

[jnm2]

Ohh... You mean that fixed (char* windowNameChars = new char[bufferSize]) is an implicit stack allocation even without an explicit statement?

Oops, I'm sorry! I misread that. Yes, @AArnott's suggestion is safe. I would tend towards using it because we don't have to think as hard about getting it right, and there's a cost to additional code complexity.

[netcorefan1]

Oh, that's good! Getting title and/or class name are the main operations performed when dealing with Windows and it's very important to know what not to do. Over the time I have realized that Win32 has many hidden traps like this and I have been able to discover them only on production.
Just for completeness, we've only talked about windows captions. Does the two risks mentioned above also applies when getting class names?