Today, find_proxy_manager() may return a proxy manager that is about to
be freed. This happens when the proxy manager's reference count reaches
zero, but proxy_manager_destroy() has not removed the proxy manager from
the apartment proxy list.
Fix this by incrementing the reference count only if it is already
nonzero. If the reference count is zero, any reference to the proxy
manager will become invalid after the current thread leaves the critical
section (apt->cs). This is because proxy_manager_destroy() will proceed
to destroy the proxy manager after the apartment lock (apt->cs) is
released.
An alternative solution would be to prevent find_proxy_manager from
observing the zero reference count in the first place. Multiple
approaches have been considered for implementing this solution, but were
eventually dropped due to several disadvantages when applied to the
current Wine codebase:
1. Always acquire the apartment lock from the proxy manager's Release()
method, and hold the lock until the proxy manager is completely
removed from the list if the reference count drops to zero.
This requires handling the special case when the proxy manager's
parent apartment has been destroyed. When an apartment is destroyed,
it sets the `parent` field of each proxy manager that was previously
owned by the apartment to NULL. This means that each access to
`This->parent->cs` has to be guarded by a NULL check for
`This->parent`.
Even if `parent` were never NULL, unconditionally acquiring a lock
may unnecessarily block the subroutine and introduce contention.
2. Opportunistically decrement the reference count without holding the
lock, but only if the count is greater than 1. This approach is
still not free from the NULL parent apartment problem.
3. Check for any concurrent reference count change from
proxy_manager_destroy(), and bail out if such change has occurred.
This makes it possible for the proxy manager's AddRef() method to
return 1, which is unusual.
Today, find_proxy_manager() tests if AddRef() returns 0 in an attempt to
protect against a race condition bug.
Note that AddRef does not return zero in normal circumstances, since
AddRef returns the reference count after the increment, not before.
Fix this by comparing the return value of AddRef() against 1, not 0.
Now that MTA objects across processes share a value of 0 in the Data2
field of the stub manager IPID, make sure that we only search the MTA
for stub managers that reside in the current process.
Signed-off-by: Connor McAdams <cmcadams@codeweavers.com>
It is possible for a thread that creates an MTA to call
CoUninitialize() and not destroy the MTA if another thread has
entered the MTA in the meantime. If the original thread then creates
an STA, subsequent attempts to find the MTA with 'apartment_findfromtid'
will get the original thread's STA. To avoid this, don't set a TID value
in the stub manager IPID value to indicate that the stub resides in the
MTA.
Signed-off-by: Connor McAdams <cmcadams@codeweavers.com>
Based on implementation of create_surrogate_server().
This patch makes 64-bit application work with its own shipped 32-bit COM server.
Signed-off-by: Dmitry Timoshkov <dmitry@baikal.ru>
Signed-off-by: Huw Davies <huw@codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard@winehq.org>
Put the string buffer at the end of the struct to
match the Windows behaviour and avoid unnecessary
pointer arithmetic.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51017
Signed-off-by: Bernhard Kölbl <besentv@gmail.com>
Signed-off-by: Huw Davies <huw@codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard@winehq.org>
Also move the reference flag to the hstring_header struct.
Signed-off-by: Bernhard Kölbl <besentv@gmail.com>
Signed-off-by: Huw Davies <huw@codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard@winehq.org>
The goal of these tests is to show, that the memory layout
of hstring_private is different on Windows, than currently
used in Wine. This creates issues with the WinRT SDK, which
allocates HSTRINGs without using the functions provided by
the combase dll.
Signed-off-by: Bernhard Kölbl <besentv@gmail.com>
Signed-off-by: Huw Davies <huw@codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard@winehq.org>
CoIncrementMTAUsage() always creates the MTA if it doesn't already exist,
and mta->tid may accidently match the apt->tid of the apartment-threaded
apartment of the thread.
Signed-off-by: Dmitry Timoshkov <dmitry@baikal.ru>
Signed-off-by: Huw Davies <huw@codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard@winehq.org>
