On Tue, Apr 19, 2005 at 01:13:12PM +0200, Alexandre Julliard wrote:
You can't hold the critsection when adding the handler since this will grab the vectored_handlers critsection, and thus acquire the sections in the reverse order of what happens when the handler is called.
Good point, I haven't even considered it. Thanks for catching this nasty. As for the list, I'm not sure it's worth sorting, that optimizes the slow path anyway. Maybe it's more advantageous to keep the most recently used DIB at the front of the list. If this is a problem, we can have a more complicated struture to do a more clever search, but this is a different patch.
ChangeLog Use vectored exceptions to get rid of VIRTUAL_SetFaultHandler().
Index: dlls/ntdll/ntdll.spec =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/ntdll.spec,v retrieving revision 1.177 diff -u -p -r1.177 ntdll.spec --- dlls/ntdll/ntdll.spec 16 Apr 2005 11:19:27 -0000 1.177 +++ dlls/ntdll/ntdll.spec 18 Apr 2005 11:42:07 -0000 @@ -1036,4 +1036,3 @@ @ cdecl MODULE_DllThreadAttach(ptr) @ cdecl MODULE_GetLoadOrderW(ptr wstr wstr) @ cdecl VERSION_Init(wstr) -@ cdecl VIRTUAL_SetFaultHandler(ptr ptr ptr) Index: dlls/ntdll/virtual.c =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/virtual.c,v retrieving revision 1.46 diff -u -p -r1.46 virtual.c --- dlls/ntdll/virtual.c 22 Feb 2005 19:33:50 -0000 1.46 +++ dlls/ntdll/virtual.c 17 Apr 2005 04:20:14 -0000 @@ -1098,25 +1104,6 @@ void virtual_init(void)
/*********************************************************************** - * VIRTUAL_SetFaultHandler - */ -BOOL VIRTUAL_SetFaultHandler( LPCVOID addr, HANDLERPROC proc, LPVOID arg ) -{ - FILE_VIEW *view; - BOOL ret = FALSE; - - RtlEnterCriticalSection( &csVirtual ); - if ((view = VIRTUAL_FindView( addr ))) - { - view->handlerProc = proc; - view->handlerArg = arg; - ret = TRUE; - } - RtlLeaveCriticalSection( &csVirtual ); - return ret; -} - -/*********************************************************************** * VIRTUAL_HandleFault */ DWORD VIRTUAL_HandleFault( LPCVOID addr ) Index: dlls/x11drv/dib.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/dib.c,v retrieving revision 1.35 diff -u -p -r1.35 dib.c --- dlls/x11drv/dib.c 14 Apr 2005 12:48:31 -0000 1.35 +++ dlls/x11drv/dib.c 19 Apr 2005 12:26:23 -0000 @@ -38,11 +38,25 @@ #include "winbase.h" #include "wingdi.h" #include "x11drv.h" +#include "excpt.h" #include "wine/debug.h"
WINE_DEFAULT_DEBUG_CHANNEL(bitmap); WINE_DECLARE_DEBUG_CHANNEL(x11drv);
+static struct list dibs_list = LIST_INIT(dibs_list); + +static CRITICAL_SECTION dibs_cs; +static CRITICAL_SECTION_DEBUG dibs_cs_debug = +{ + 0, 0, &dibs_cs, + { &dibs_cs_debug.ProcessLocksList, &dibs_cs_debug.ProcessLocksList }, + 0, 0, { 0, (DWORD)(__FILE__ ": dibs_cs") } +}; +static CRITICAL_SECTION dibs_cs = { &dibs_cs_debug, -1, 0, 0, 0, 0 }; + +PVOID dibs_handler = 0; + static int ximageDepthTable[32];
/* This structure holds the arguments for DIB_SetImageBits() */ @@ -4268,22 +4282,43 @@ static void X11DRV_DIB_DoUpdateDIBSectio /*********************************************************************** * X11DRV_DIB_FaultHandler */ -static BOOL X11DRV_DIB_FaultHandler( LPVOID res, LPCVOID addr ) +static LONG CALLBACK X11DRV_DIB_FaultHandler( PEXCEPTION_POINTERS ep ) { - X_PHYSBITMAP *physBitmap = res; - INT state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + X_PHYSBITMAP *physBitmap = NULL; + BOOL found = FALSE; + const BYTE *addr; + struct list *ptr;
- if (state != DIB_Status_InSync) { - /* no way to tell whether app needs read or write yet, - * try read first */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); - } else { - /* hm, apparently the app must have write access */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); - } - X11DRV_DIB_Unlock( physBitmap, TRUE ); + if (ep->ExceptionRecord->ExceptionCode != EXCEPTION_ACCESS_VIOLATION) + return EXCEPTION_CONTINUE_SEARCH; + + addr = (const BYTE*)ep->ExceptionRecord->ExceptionInformation[1]; + + EnterCriticalSection(&dibs_cs); + LIST_FOR_EACH( ptr, &dibs_list ) + { + physBitmap = LIST_ENTRY( ptr, X_PHYSBITMAP, entry ); + if ((physBitmap->base <= addr) && (addr < physBitmap->base + physBitmap->size)) + { + found = TRUE; + break; + } + } + LeaveCriticalSection(&dibs_cs); + + if (!found) return EXCEPTION_CONTINUE_SEARCH; + + X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + if (ep->ExceptionRecord->ExceptionInformation[0]) { + /* the app tried to write the DIB bits */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); + } else { + /* the app tried to read the DIB bits */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); + } + X11DRV_DIB_Unlock( physBitmap, TRUE );
- return TRUE; + return EXCEPTION_CONTINUE_EXECUTION; }
/*********************************************************************** @@ -4566,7 +4601,6 @@ static XImage *X11DRV_XShmCreateImage( i HBITMAP X11DRV_CreateDIBSection( X11DRV_PDEVICE *physDev, HBITMAP hbitmap, const BITMAPINFO *bmi, UINT usage ) { - extern BOOL VIRTUAL_SetFaultHandler(LPCVOID addr, BOOL (*proc)(LPVOID, LPCVOID), LPVOID arg); X_PHYSBITMAP *physBitmap; DIBSECTION dib;
@@ -4602,11 +4636,18 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_
/* install fault handler */ InitializeCriticalSection( &physBitmap->lock ); - if (VIRTUAL_SetFaultHandler(dib.dsBm.bmBits, X11DRV_DIB_FaultHandler, physBitmap)) - { - X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE ); - physBitmap->status = DIB_Status_AppMod; - } + + physBitmap->base = dib.dsBm.bmBits; + physBitmap->size = dib.dsBmih.biSizeImage; + physBitmap->status = DIB_Status_AppMod; + + if (!dibs_handler) + dibs_handler = AddVectoredExceptionHandler( TRUE, X11DRV_DIB_FaultHandler ); + EnterCriticalSection( &dibs_cs ); + list_add_head( &dibs_list, &physBitmap->entry ); + LeaveCriticalSection( &dibs_cs ); + + X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE );
return hbitmap; } @@ -4616,6 +4657,10 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ */ void X11DRV_DIB_DeleteDIBSection(X_PHYSBITMAP *physBitmap, DIBSECTION *dib) { + EnterCriticalSection( &dibs_cs ); + list_remove( &physBitmap->entry ); + LeaveCriticalSection( &dibs_cs ); + if (dib->dshSection) X11DRV_DIB_Coerce(physBitmap, DIB_Status_InSync, FALSE);
Index: dlls/x11drv/x11drv.h =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv.h,v retrieving revision 1.69 diff -u -p -r1.69 x11drv.h --- dlls/x11drv/x11drv.h 14 Apr 2005 12:48:11 -0000 1.69 +++ dlls/x11drv/x11drv.h 19 Apr 2005 01:44:27 -0000 @@ -64,6 +64,7 @@ typedef int Status; #include "winuser.h" #include "ddrawi.h" #include "thread.h" +#include "wine/list.h"
#define MAX_PIXELFORMATS 8
@@ -111,6 +112,9 @@ typedef struct #ifdef HAVE_LIBXXSHM XShmSegmentInfo shminfo; /* shared memory segment info */ #endif + struct list entry; /* Entry in global DIB list */ + const BYTE *base; /* Base address */ + SIZE_T size; /* Size in bytes */ } X_PHYSBITMAP;
/* X physical font */ @@ -147,6 +151,7 @@ typedef struct /* GCs used for B&W and color bitmap operations */ extern GC BITMAP_monoGC, BITMAP_colorGC; extern X_PHYSBITMAP BITMAP_stock_phys_bitmap; /* phys bitmap for the default stock bitmap */ +extern PVOID dibs_handler;
#define BITMAP_GC(physBitmap) (((physBitmap)->pixmap_depth == 1) ? BITMAP_monoGC : BITMAP_colorGC)
Index: dlls/x11drv/x11drv_main.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv_main.c,v retrieving revision 1.102 diff -u -p -r1.102 x11drv_main.c --- dlls/x11drv/x11drv_main.c 14 Apr 2005 12:48:11 -0000 1.102 +++ dlls/x11drv/x11drv_main.c 18 Apr 2005 12:06:15 -0000 @@ -422,6 +422,9 @@ static void process_detach(void) /* cleanup GDI */ X11DRV_GDI_Finalize();
+ /* cleanup DIB handling */ + if (dibs_handler) RemoveVectoredExceptionHandler( dibs_handler ); + DeleteCriticalSection( &X11DRV_CritSection ); }