- Wine-devel - list.winehq.org

Refcounting in SelectObject
by Uwe Bonnes 17 Aug '01

17 Aug '01

Hallo, we discussed this January 2001 with no final outcome. Some application (gcprevue version 9) does 080673a0:Call gdi32.SelectObject(000011d4,000011d8) ret=5f485bb1 080673a0:Ret gdi32.SelectObject() retval=00000044 ret=5f485bb1 080673a0:Call gdi32.DeleteObject(000011d8) ret=5f487813 080673a0:Ret gdi32.DeleteObject() retval=00000001 ret=5f487813 080673a0:Call gdi32.BitBlt(000011d4,00000000,00000000,0000008c,000001a7,000009d0,00000000,00000000,00cc0020) ret=0048d1e6 and gets an XErrorEvent and fails to Debugbreak. Starting theapplication with winedbg still gets this error, but DebugBreak isn't executed and the application proceeds. The Windows Api explicitly tells for DeleteObject: Return Value: ... If the specified handle is not valid or is currently selected into a device context, the return value is FALSE. Windows Internals tells about some elements in GDIOBJHDR in debugbuild of windows and has pseudo code for incrementing some refrence counters in SelectObject. Appended patch tries to implement this scheme. The original discussion dissolved into nothing after Andi saw the need for such refcounting, but also mentioned possible resource leakage. Appended code tries to clean up better by: - Remembering an unsuccessfull DeleteObject by adding 0x8000 to the Refcount - Call DeleteObject in SelectObject when the RefCount of the released handle is 0x8000 - Decrementing the RefCount when a hBitmap is still selected into a DC when DeleteDC is called for that DC - Call DeleteObject in DeleteDC when RefCount is 0x8000 For some applications I tested, I always saw the delayed DeleteObject done after the refused DeleteObject in the first place. Is this approach better? Uwe Bonnes bon(a)elektron.ikp.physik.tu-darmstadt.de Free Software: If you contribute nothing, expect nothing -- Index: wine/include/gdi.h =================================================================== RCS file: /home/wine/wine/include/gdi.h,v retrieving revision 1.50 diff -u -r1.50 gdi.h --- wine/include/gdi.h 2001/08/15 23:33:20 1.50 +++ wine/include/gdi.h 2001/08/16 23:23:12 @@ -45,6 +45,9 @@ HANDLE16 hNext; WORD wMagic; DWORD dwCount; + WORD wMetaList; + WORD wSelCount;/* only in debug builds of windows */ + WORD wObjTask;/* only in debug builds of windows */ } GDIOBJHDR; Index: wine/objects/gdiobj.c =================================================================== RCS file: /home/wine/wine/objects/gdiobj.c,v retrieving revision 1.54 diff -u -r1.54 gdiobj.c --- wine/objects/gdiobj.c 2001/08/16 19:13:52 1.54 +++ wine/objects/gdiobj.c 2001/08/16 23:23:13 @@ -374,6 +374,7 @@ obj->hNext = 0; obj->wMagic = magic|OBJECT_NOSYSTEM; obj->dwCount = ++count; + obj->wSelCount = 0; TRACE_SEC( *handle, "enter" ); return obj; @@ -545,6 +546,15 @@ GDI_ReleaseObj( obj ); return TRUE; } + + if(header->wSelCount) + { + TRACE("delayed for %04x because object in use, count %d\n", + obj,header->wSelCount); + header->wSelCount+=0x8000; /* mark for delete */ + GDI_ReleaseObj( obj ); + return FALSE; + } TRACE("%04x\n", obj ); @@ -814,12 +824,32 @@ HGDIOBJ WINAPI SelectObject( HDC hdc, HGDIOBJ handle ) { HGDIOBJ ret = 0; + GDIOBJHDR *header; + DC * dc = DC_GetDCUpdate( hdc ); if (!dc) return 0; TRACE("hdc=%04x %04x\n", hdc, handle ); if (dc->funcs->pSelectObject) ret = dc->funcs->pSelectObject( dc, handle ); GDI_ReleaseObj( hdc ); + if (ret) + { + if (!(header = GDI_GetObjPtr( handle, MAGIC_DONTCARE ))) return 0; + header->wSelCount++; + GDI_ReleaseObj(handle); + if (!(header = GDI_GetObjPtr( ret, MAGIC_DONTCARE ))) return 0; + if(header->wSelCount) + header->wSelCount--; + if(header->wSelCount == 0x8000) /* handle delayed DeleteObject*/ + { + header->wSelCount = 0; + GDI_ReleaseObj(ret); + TRACE("executing delayed DeleteObject for %04x\n",ret); + DeleteObject(ret); + } + else + GDI_ReleaseObj(ret); + } return ret; } Index: wine/objects/dc.c =================================================================== RCS file: /home/wine/wine/objects/dc.c,v retrieving revision 1.62 diff -u -r1.62 dc.c --- wine/objects/dc.c 2001/08/16 19:01:23 1.62 +++ wine/objects/dc.c 2001/08/16 23:23:14 @@ -766,6 +766,25 @@ if (dc->hGCClipRgn) DeleteObject( dc->hGCClipRgn ); if (dc->pAbortProc) THUNK_Free( (FARPROC)dc->pAbortProc ); if (dc->hookThunk) THUNK_Free( (FARPROC)dc->hookThunk ); + if (dc->hBitmap) + { + GDIOBJHDR * header; + if ((header = GDI_GetObjPtr( dc->hBitmap, MAGIC_DONTCARE ))) + { + if(header->wSelCount ) + header->wSelCount--; + if(header->wSelCount == 0x8000) + { + header->wSelCount = 0; + GDI_ReleaseObj(dc->hBitmap); + TRACE("executing delayed DeleteObject for %04x\n",dc->hBitmap); + DeleteObject(dc->hBitmap); + } + else + GDI_ReleaseObj(dc->hBitmap); + + } + } PATH_DestroyGdiPath(&dc->path); GDI_FreeObject( hdc, dc );

1 0

Re: DC lock in CreateDIBitmap
by Alexandre Julliard 17 Aug '01

17 Aug '01

Uwe Bonnes <bon(a)elektron.ikp.physik.tu-darmstadt.de> writes: > CreateDIBitmap is called with the HDC lock hold, and when the (fColor) clause > is true, CreateDCA() checks that the lock is not hold. A laymans approach to > release hdc before CreateDCA() and reopen it afterwards resulted in this > report asr startup, with no visual output: I don't think we should be using CreateDC at all in CreateDIBitmap. Could you try this? Index: objects/dib.c =================================================================== RCS file: /opt/cvs-commit/wine/objects/dib.c,v retrieving revision 1.59 diff -u -r1.59 dib.c --- objects/dib.c 2001/08/15 23:33:20 1.59 +++ objects/dib.c 2001/08/16 21:08:12 @@ -875,12 +875,7 @@ /* Now create the bitmap */ - if (fColor) - { - HDC tmpdc = CreateDCA( "DISPLAY", NULL, NULL, NULL ); - handle = CreateCompatibleBitmap( tmpdc, width, height ); - DeleteDC( tmpdc ); - } + if (fColor) handle = CreateCompatibleBitmap( hdc, width, height ); else handle = CreateBitmap( width, height, 1, 1, NULL ); if (!handle) return 0; -- Alexandre Julliard julliard(a)winehq.com

2 1

Problems building with extern inline functions in winebase.h
by Mike Bond 17 Aug '01

17 Aug '01

A couple days ago a number of inline functions were put into winebase.h. Since then I have been unable to build, using gcc 2.96-81 (RedHat). Apparently this version of gcc thinks that because there is an extern prefixing the inline, even though the function does in fact have a body, that the function really is external. Removing the extern, just having inline as one would do in C++, does not appear to work correctly either, and causes the function to appear as a exported symbol in all the object files that header gets included with, thus resulting in multiple definitions. I'm not sure what the correct solution would be, other than using the older function call method for this version of gcc. Perhaps I'm just missing some variant on the use of inline? -- TTFN MikeB

5 5

Re: patch.py?id=997918367324549717828631
by Alexandre Julliard 17 Aug '01

17 Aug '01

lawson_whitney(a)juno.com writes: > I can't remember that I've ever seen the fixme that was here before, > but I'll check again when I send this. No, the CheckNotSysLevel calls were not done before. I added a few of them in GDI to find potential locking bugs; seems pretty successful so far ;-) I think this should fix your problem: Index: objects/dc.c =================================================================== RCS file: /opt/cvs-commit/wine/objects/dc.c,v retrieving revision 1.61 diff -u -r1.61 dc.c --- objects/dc.c 2001/08/15 23:33:20 1.61 +++ objects/dc.c 2001/08/16 17:34:45 @@ -524,7 +524,9 @@ success=FALSE; } GDI_ReleaseObj( hdcs ); + GDI_ReleaseObj( hdc ); DeleteDC( hdcs ); + if (!(dc = DC_GetDCPtr( hdc ))) return FALSE; } GDI_ReleaseObj( hdc ); return success; -- Alexandre Julliard julliard(a)winehq.com

2 1

Re: [resend] Don't crash when CUPS is installed
by mm＠caldera.de 17 Aug '01

17 Aug '01

In article <997995321.1690.4.camel(a)bugg> you wrote: > Hi all, > Having CUPS installed and a printer in CUPS configured would crash an > application I'm looking at. The reason was that the printer would be > added with a NULL name. > - if (!AddPrinterA(NULL,2,(LPBYTE)&pinfo2a)) { > + if (!AddPrinterA(printers[i],2,(LPBYTE)&pinfo2a)) { This will just make the printer not added AT ALL, since there is the following code in AddPrinterW: if(pName != NULL) { ERR("pName = %s - unsupported\n", debugstr_w(pName)); SetLastError(ERROR_INVALID_PARAMETER); return 0; } Please do not apply. Ciao, Marcus

1 0

DC lock in CreateDIBitmap
by Uwe Bonnes 17 Aug '01

17 Aug '01

Hallo, with the new GDI_CheckNotLock() stuff, some application (gcprevue 9) fails with > err:win32:_CheckNotSysLevel Holding lock 0x4079b9e8 level 3 Backtrace is: Backtrace: =>0 0x400e21d1 (_CheckNotSysLevel+0x51(lock=0x4079b9e8) [syslevel.c:152] in libntdll.so) (ebp=405c6090) 1 0x4078261c (GDI_CheckNotLock+0x1c [gdiobj.c:510] in libgdi32.so) (ebp=405c60a0) 2 0x4077823d (CreateDCA+0x1d(driver=0x407909d3, device=0x0, output=0x0, initData=0x0, s=0x0, s=0x0, s=0x407909d3) [dc.c:558] in libgdi32.so) (ebp=405c61e8) 3 0x4077bbc6 (CreateDIBitmap+0x166(hdc=0xdec, header=0x41eadb50, init=0x0, bits=0x0, data=0x41eadb50, coloruse=0x0) [dib.c:881] in libgdi32.so) (ebp=405c6218) 4 0x409f9479 (X11DRV_DIB_CreateDIBSection+0x359(dc=0x404225f4, bmi=0x41eadb50, usage=0x0, bits=0x411000d8, section=0x0, offset=0x0, ovr_pitch=0x0) [dib.c:3914] in libx11drv.so) (ebp=405c62a0) 5 0x4077bd75 (DIB_CreateDIBSection+0x65(hdc=0xdec, bmi=0x41eadb50, usage=0x0, bits=0x411000d8, section=0x0, offset=0x0, ovr_pitch=0x0) [dib.c:944] in libgdi32.so) (ebp=405c62d4) 6 0x4077bdd1 (CreateDIBSection+0x31(hdc=0xdec, bmi=0x41eadb50, usage=0x0, bits=0x411000d8, section=0x0, offset=0x0) [dib.c:961] in libgdi32.so) (ebp=405c6304) 7 0x00401fba (gcprevue.exe..text+0xfba in D:\cae\gcp9.0.0.8\gcprevue.exe) (ebp=ffffffff) CreateDIBitmap is called with the HDC lock hold, and when the (fColor) clause is true, CreateDCA() checks that the lock is not hold. A laymans approach to release hdc before CreateDCA() and reopen it afterwards resulted in this report asr startup, with no visual output: fixme:win32:PE_CreateModule Security directory ignored err:win32:_LeaveSysLevel (0x4079ba08, level 3): Invalid state: count 0 mutex (nil). err:ntdll:RtlpWaitForCriticalSection section 0x4010fb88 "rtl.c: peb_lock" wait timed out, retrying (60 sec) fs=030f err:ntdll:RtlpWaitForCriticalSection section 0x4010fb88 "rtl.c: peb_lock" wait timed out, retrying (60 sec) fs=031f err:ntdll:RtlpWaitForCriticalSection section 0x4079ba08 "gdiobj.c: GDI_level" wait timed out, retrying (60 sec) fs=008f How to cure this problem? Bye Uwe Bonnes bon(a)elektron.ikp.physik.tu-darmstadt.de Free Software: If you contribute nothing, expect nothing --

1 0

Stupid Winedbg question
by Bobby Bingham 16 Aug '01

16 Aug '01

Ok, I feel like an idiot for having to ask this, but it's my first time realing using the debugger. When wine starts the debugger on an exception, it opens it up in another xterm. Is there an easy way to copy it's output? The usual select and middle-click isn't working because it doesn't let me select the text to begin with. Thanks in advance for your patience. Bobby Bingham

3 2

Re: Extra space at bottom of toolbar
by Guy L. Albertelli 16 Aug '01

16 Aug '01

-----Original Message----- From: Bobby Bingham <uhmmmm(a)ameritech.net> To: Wine Devel <wine-devel(a)winehq.com> Date: Thursday, August 16, 2001 12:28 AM Subject: Extra space at bottom of toolbar >Has anyone else seen this... In WinZip 8.0 (I don't know about other >versions/software), there are about 6 extra pixels at the bottom of the >toolbar below the buttons. Does anyone know what's causing this? I can look >into it if I get some time, but I was wondering if anybody knew. > >Bobby Bingham > > The current CVS shows the following differences between the builtin control and the native one: 1. Button top-left corner is 2 pixels too high. 2. Button is 2 pixels too short. 3. Background bar is 2 pixels too wide. This gives the 6 pixels you noticed. Items 1 & 2 are definitely within the toolbar.c. Item 3 may be in either toolbar or rebar. I need to work on moving screenshots from Win to Linux for comparison. Guy

1 0

Re: Protocol for adding FIXMEs
by Alexandre Julliard 16 Aug '01

16 Aug '01

"Bill Medland" <medbi01(a)accpac.com> writes: > However I am occasionally frustrated by the number of times I see the same > FIXME (e.g. the pthread_rwlock* and the "kill other threads" one). I also > recognise from posts that these disconcert newbies whereas after a couple > of weeks one gets used to them. > > So maybe it is better to use FIXME comments where one knows that something > is wrong but it doesn't seem to matter. FIXME macros should be used in cases where we know something is wrong but we don't expect this part of the code to ever get used. This way if some app triggers the wrong code we can hear about it. It doesn't make sense to add FIXME output to parts of the code we know are going to be called, it just generates noise; a comment is much better in this case. > (For example the WM_NCCALCSIZE message uses wParam as a Boolean value and > if it is TRUE then the lParam no longer points to a simple rectangle but to > a structure whose first element is a rectangle. Very little if any code > handles that case explicitly.) I don't think this specific case is a FIXME at all. Apps are allowed to ignore the extra information if they don't have any use for it. -- Alexandre Julliard julliard(a)winehq.com

1 0

_CheckNotSysLevel (was: patch.py?id=997918367324549717828631)
by lawson_whitney＠juno.com 16 Aug '01

16 Aug '01

On Thu, 16 Aug 2001 whit(a)giftie.alien.org wrote: > Good day! > > I can't remember that I've ever seen the fixme that was here before, > but I'll check again when I send this. > Is that a new fixme? I reversed just this patch and there are more of this fixme than any other. Hope the backtrace I sent before is useful. Regards, Lawson ---oof--- ________________________________________________________________ GET INTERNET ACCESS FROM JUNO! Juno offers FREE or PREMIUM Internet access for less! Join Juno today! For your FREE software, visit: http://dl.www.juno.com/get/tagj.

1 0