Bug #6439: requesting explanation of GDI_CheckNotLock in order to fix

[Alex Villací­s Lasso]

Vitaliy Margolen escribió:
> Alex Villací­s Lasso wrote:
>   
>> I would like to draw attention to bug #6439
>>     
> Why that bug in particular? Can we pick any other? 
>   
Yes, you can:

http://bugs.winehq.org/buglist.cgi?query_format=&short_desc_type=allwordssubstr&short_desc=&long_desc_type=substring&long_desc=_CheckNotSysLevel&bug_file_loc_type=allwordssubstr&bug_file_loc=&keywords_type=allwords&keywords=&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&votes=&changedin=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Bug+Number&field0-0-0=noop&type0-0-0=noop&value0-0-0=

This search lists 11 (unfixed) bugs that mention _CheckNotSysLevel.

Sorry if I sounded like a newbie, but the question was made in order to 
avoid sending a patch that would be immediately rejected with "your fix 
is incorrect, this should be fixed in xyz way at foo.c ..." The only 
users of _CheckNotSysLevel in the Wine tree are dlls/user and dll/gdi . 
In dlls/user the pattern seems to be to use USER_Lock() and 
USER_Unlock() in strict pairs to protect blocks of code and not call any 
other functions that use USER_CheckNotLock(). In dlls/gdi the lock is 
taken once every time a reference to a GDI object is requested, but the 
rule about not calling other functions that check GDI_CheckNotLock() can 
be violated (at least in my bug) on the rendering of a metafile, as 
shown by the sequence:

(App tries to draw an icon into a metafile)
DrawIconEx calls into StretchBlt
(dlls/gdi/bitblt.c:140) StretchBlt calls DC_GetDCUpdate, then 
DC_GetDCPtr . Both of these functions increment the GDI lock, so it ends 
up with a lock count of (at least) 2.
(dlls/gdi/bitblt.c:148) With lock held, StretchBlt calls into 
dcDst->funcs->pStretchBlt. Since this is a metafile, it calls into 
EMFDRV_StretchBlt
(dlls/gdi/enhmfdrv/bitblt.c:186) EMFDRV_StretchBlt delegates to 
EMFDRV_BitBlockTransfer
(dlls/gdi/enhmfdrv/bitblt.c:163) EMFDRV_BitBlockTransfer calls into 
GetDIBits <-- possible source of bug?
(dlls/gdi/dib.c:556) GetDIBits calls CreateCompatibleDC with the source HDC
(dlls/gdi/dc.c:728) CreateCompatibleDC() calls into GDI_CheckNotLock 
unconditionally. But StretchBlt already raised the lock count --> debug 
assertion.

So, when metafiles are involved, this sequence of calls triggers the 
debug assertion in CreateCompatibleDC(). Since I don't really see the 
point of having the assertion in CreateCompatibleDC(), I was planning to 
send a patch to simply remove the assertion. However, the assertion must 
be there for a reason (debug a deadlock, maybe), so I thought I would 
check with others with more experience in GDI. If this assertion has to 
stay, then the metafile functions should be reworked to remove calls to 
GetDIBits or any other GDI calls that check the assertion. In fact, any 
GDI call that uses CreateDC[AW], CreateCompatibleDC or DeleteDC must not 
be called with the GDI lock held, because of the debug assertion.

-- 
The following cryptic message was allegedly found in the inner edge of a Windows
XP installation CD:

4F6E65204F5320746F2072756C65207468656D20616C6C2C204F6E65204F5320746F2066696E6420
7468656D2C0D0A4F6E65204F5320746F206272696E67207468656D20616C6C20616E6420696E2074
6865206461726B6E6573732062696E64207468656D2E0A

It is rumored that only a true Unix Wizard can decypher this mysterious message,
which supposedly encodes the true nature and purpose of the software.