[PATCH v2] server: Track desktop users per thread instead of per process.

[Rémi Bernon]

On 4/20/21 10:51 PM, Alexandre Julliard wrote:
> Rémi Bernon <rbernon at codeweavers.com> writes:
> 
>> As some thread may use a different desktop from their process.
>>
>> This fixes the user32 win tests, which leaks a desktop that never gets
>> closed. The test_shell_window test creates a new desktop, which spawns
>> explorer.exe process, incrementing the desktop user count to 1, then
>> associates the desktop to a thread, which closes it on exit.
>>
>> Never the user count is incremented to 2, and closing the thread desktop
>> doesn't either check whether the desktop process should be terminated.
>>
>> Reversely, it is possible to create a desktop, associate it with a
>> thread /and/ a process, and this time the desktop process would be
>> terminated when the process exits, although the thread may still be
>> using it.
>>
>> Tracking the users per thread is more robust and fixes the problem as
>> set_thread_desktop increments the desktop user count, and thread exit
>> decrements it.
>>
>> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
>> ---
>>
>> v2: Close the system process threads desktops before setting the
>>      is_system flag.
>>
>>   server/process.c    |  5 +++-
>>   server/thread.c     | 10 +++++++-
>>   server/user.h       |  2 ++
>>   server/winstation.c | 62 +++++++++++++++++++++++++++------------------
>>   4 files changed, 52 insertions(+), 27 deletions(-)
> 
> This breaks the tests here:
> 
> tools/runtest -q -P wine -T . -M services.exe -p programs/services/tests/services.exe_test.exe service && touch programs/services/tests/service.ok
> service.c:422: Test failed: service: GetDesktopWindow returned invalid window 00000000
> service.c:422: Test failed: service: CreateWindow failed: 1411
> service.c:422: Test failed: service: style = 0, expected visible
> service.c:422: Test failed: service: ShowWindow returned 0
> service.c:422: Test failed: service: DestroyWindow failed: 00000005
> make: *** [Makefile:157374: programs/services/tests/service.ok] Error 5
> 

Interesting... the problem is that we now reset the thread desktop 
handle to 0 when making system processes, were we were previously only 
messing with the process (default) desktop handle.

Interestingly, I also saw that this could be reversed later, as the 
process default desktop can be set after make_process_system was called 
already, restoring process and thread desktop handles to non-0 values.

I'm not completely sure but I think we can consider releasing only the 
desktop user reference count when making a process system, without 
touching the thread (or the process) desktop handles?

Threads would keep a handle to a possibly already terminated desktop, 
but I think it's already the case when the process default desktop 
handle is later changed.
-- 
Rémi Bernon <rbernon at codeweavers.com>