Message du 11/01/05 08:45 De : "Paul Vriens" A : "Bill Medland" Copie à : "wine-devel@winehq.org" Objet : Re: Correction to COINIT_MULTITHREADED test On Mon, 2005-01-10 at 21:20, Bill Medland wrote:
Bill Medland (billmedland@mercuryspeed.com) Correct testing for multithreaded (as spotted by Paul Vriens; fix suggested by Robert Shearman)
Hi Bill,
it wasn't the check that was way over my head. The thing is, that with this code change we walk a new path in COM_CreateApartment.
I don't know what the result is but in CoInitializeEx we do:
545 if (!(apt = COM_CurrentInfo()->apt)) 546 { 547 apt = COM_CreateApartment(dwCoInit); 548 if (!apt) return E_OUTOFMEMORY; 549 }
and apparently that leaves us with a E_OUTOFMEMORY all the time. Which means that we will not come to the part:
All the time ? Looking at COM_CreateApartment it seems COINIT_APARTMENTTHREADED works but indeed COINIT_MULTITHREADED always return NULL. I think the code at the beginning of COM_CreateApartment is wrong.
TRACE("thread 0x%lx is entering the multithreaded apartment\n", GetCurrentThreadId()); COM_CurrentInfo()->apt = &MTA; return apt;
I think we should return COM_CurrentInfo()->apt rather than apt.
Bye, Christian
On Tue, 2005-01-11 at 12:19, Christian Costa wrote:
All the time ? Looking at COM_CreateApartment it seems COINIT_APARTMENTTHREADED works but indeed COINIT_MULTITHREADED always return NULL. I think the code at the beginning of COM_CreateApartment is wrong.
TRACE("thread 0x%lx is entering the multithreaded apartment\n", GetCurrentThreadId()); COM_CurrentInfo()->apt = &MTA; return apt;I think we should return COM_CurrentInfo()->apt rather than apt.
Bye, Christian
With all the time, I mean that when CoInitializeEx is called with COINIT_MULITHREADED it itself will return E_OUTOFMEMORY (because of the NULL being passed back by COM_CreateApartment).
Another thing in MSDN which is currently not accounted for (AFAIK) is:
Multiple calls to CoInitializeEx by the same thread are allowed as long as they pass the same concurrency flag, but subsequent valid calls return S_FALSE
Cheers,
Paul.
Paul Vriens wrote:
On Tue, 2005-01-11 at 12:19, Christian Costa wrote:
All the time ? Looking at COM_CreateApartment it seems COINIT_APARTMENTTHREADED works but indeed COINIT_MULTITHREADED always return NULL. I think the code at the beginning of COM_CreateApartment is wrong.
TRACE("thread 0x%lx is entering the multithreaded apartment\n", GetCurrentThreadId()); COM_CurrentInfo()->apt = &MTA; return apt;I think we should return COM_CurrentInfo()->apt rather than apt.
Bye, Christian
With all the time, I mean that when CoInitializeEx is called with COINIT_MULITHREADED it itself will return E_OUTOFMEMORY (because of the NULL being passed back by COM_CreateApartment).
Ah! Ok! :-)
Another thing in MSDN which is currently not accounted for (AFAIK) is:
Multiple calls to CoInitializeEx by the same thread are allowed as long as they pass the same concurrency flag, but subsequent valid calls return S_FALSE
You're right. The current code does not handle that. I've just sent a patch to fix it.
Bye, Christian
On Tue, 2005-01-11 at 12:19, Christian Costa wrote:
All the time ? Looking at COM_CreateApartment it seems COINIT_APARTMENTTHREADED works but indeed COINIT_MULTITHREADED always return NULL. I think the code at the beginning of COM_CreateApartment is wrong.
TRACE("thread 0x%lx is entering the multithreaded apartment\n", GetCurrentThreadId()); COM_CurrentInfo()->apt = &MTA; return apt;I think we should return COM_CurrentInfo()->apt rather than apt.
Bye, Christian
Just another thing.
shouldn't s_COMLockCount be a per thread thing? Currently it's global.
What I personally find strange is the behavior of InterlockedExchangeAdd especially with respect to the comments in compobj.c:
/* * Check the lock count. If this is the first time going through the initialize * process, we have to initialize the libraries. * * And crank-up that lock count. */ lCOMRefCnt = InterlockedExchangeAdd(&s_COMLockCount,1); TRACE("s_COMLockCount is now: %08lx\n", lCOMRefCnt);
if (lCOMRefCnt==0)
and
/* * Decrease the reference count. * If we are back to 0 locks on the COM library, make sure we free * all the associated data structures. */ lCOMRefCnt = InterlockedExchangeAdd(&s_COMLockCount,-1); TRACE("s_COMLockCount is now: %08lx\n", lCOMRefCnt); if (lCOMRefCnt==1) {
The TRACE is something I added.
The comments say 'If this is the first time' and the check is for '==0' and 'If we are back to 0' but the check is for '==1'. A bit confusing.
Paul.
Paul Vriens wrote:
On Tue, 2005-01-11 at 12:19, Christian Costa wrote:
All the time ? Looking at COM_CreateApartment it seems COINIT_APARTMENTTHREADED works but indeed COINIT_MULTITHREADED always return NULL. I think the code at the beginning of COM_CreateApartment is wrong.
TRACE("thread 0x%lx is entering the multithreaded apartment\n", GetCurrentThreadId()); COM_CurrentInfo()->apt = &MTA; return apt;I think we should return COM_CurrentInfo()->apt rather than apt.
Bye, Christian
Just another thing.
shouldn't s_COMLockCount be a per thread thing? Currently it's global.
No. This applies to the whole process.
What I personally find strange is the behavior of InterlockedExchangeAdd especially with respect to the comments in compobj.c:
/*
- Check the lock count. If this is the first time going through the
initialize
- process, we have to initialize the libraries.
- And crank-up that lock count.
*/ lCOMRefCnt = InterlockedExchangeAdd(&s_COMLockCount,1); TRACE("s_COMLockCount is now: %08lx\n", lCOMRefCnt);
if (lCOMRefCnt==0)
and
/*
- Decrease the reference count.
- If we are back to 0 locks on the COM library, make sure we free
- all the associated data structures.
*/ lCOMRefCnt = InterlockedExchangeAdd(&s_COMLockCount,-1); TRACE("s_COMLockCount is now: %08lx\n", lCOMRefCnt); if (lCOMRefCnt==1) {
The TRACE is something I added.
The comments say 'If this is the first time' and the check is for '==0' and 'If we are back to 0' but the check is for '==1'. A bit confusing.
InterlockedExchangeAdd returns the value before (and not after like InterlockedIncrement) the value is added. You've done too much InterlockedIncrement stuff. You should do a small break. ;-) Maybe InterlockedIncrement should have been used here instead.
Bye, Christian
-
Christian Costa -
Paul Vriens