From d8eb783e69f322baf6fa03551c489a379a377aef Mon Sep 17 00:00:00 2001 From: Thomas Bluemel Date: Mon, 22 Mar 2004 20:14:29 +0000 Subject: [PATCH] fixed a couple of dead-locks in the region code svn path=/trunk/; revision=8838 --- reactos/include/win32k/region.h | 8 +- reactos/subsys/win32k/include/window.h | 2 + reactos/subsys/win32k/ntuser/painting.c | 11 ++- reactos/subsys/win32k/ntuser/winpos.c | 36 +++++++-- reactos/subsys/win32k/objects/cliprgn.c | 28 +++++-- reactos/subsys/win32k/objects/region.c | 101 +++++++++++++----------- 6 files changed, 117 insertions(+), 69 deletions(-) diff --git a/reactos/include/win32k/region.h b/reactos/include/win32k/region.h index 0588984b665..607bebffdc0 100644 --- a/reactos/include/win32k/region.h +++ b/reactos/include/win32k/region.h @@ -151,9 +151,9 @@ NtGdiGetRegionData(HRGN hrgn, HRGN STDCALL REGION_CropRgn(HRGN hDst, HRGN hSrc, const PRECT lpRect, PPOINT lpPt); -HRGN STDCALL UnsafeIntCreateRectRgnIndirect(CONST PRECT rc); -INT STDCALL UnsafeIntGetRgnBox(HRGN hRgn, LPRECT pRect); -HRGN FASTCALL UnsafeIntUnionRectWithRgn(HRGN hDest, CONST PRECT Rect); -BOOL FASTCALL UnsafeIntRectInRegion(HRGN hRgn, CONST LPRECT rc); +HRGN FASTCALL UnsafeIntCreateRectRgnIndirect(CONST PRECT rc); +INT FASTCALL UnsafeIntGetRgnBox(PROSRGNDATA Rgn, LPRECT pRect); +VOID FASTCALL UnsafeIntUnionRectWithRgn(PROSRGNDATA RgnDest, CONST PRECT Rect); +BOOL FASTCALL UnsafeIntRectInRegion(PROSRGNDATA Rgn, CONST LPRECT rc); #endif diff --git a/reactos/subsys/win32k/include/window.h b/reactos/subsys/win32k/include/window.h index 9a8ee46c634..aed7a412300 100644 --- a/reactos/subsys/win32k/include/window.h +++ b/reactos/subsys/win32k/include/window.h @@ -60,6 +60,8 @@ typedef struct _WINDOW_OBJECT /* Handle of region of the window to be updated. */ HANDLE UpdateRegion; HANDLE NCUpdateRegion; + /* Handle of the window region. */ + HANDLE WindowRegion; /* Lock to be held when manipulating (NC)UpdateRegion */ FAST_MUTEX UpdateLock; /* Pointer to the owning thread's message queue. */ diff --git a/reactos/subsys/win32k/ntuser/painting.c b/reactos/subsys/win32k/ntuser/painting.c index f5240746ca2..910fb6940e8 100644 --- a/reactos/subsys/win32k/ntuser/painting.c +++ b/reactos/subsys/win32k/ntuser/painting.c @@ -16,7 +16,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. * - * $Id: painting.c,v 1.76 2004/03/13 17:33:53 gvg Exp $ + * $Id: painting.c,v 1.77 2004/03/22 20:14:29 weiden Exp $ * * COPYRIGHT: See COPYING in the top level directory * PROJECT: ReactOS kernel @@ -212,12 +212,12 @@ IntInvalidateWindows(PWINDOW_OBJECT Window, HRGN hRgn, ULONG Flags) * Clip the given region with window rectangle (or region) */ -#ifdef TODO + IntLockWindowUpdate(Window); if (!Window->WindowRegion) -#endif { HRGN hRgnWindow; + IntUnLockWindowUpdate(Window); hRgnWindow = UnsafeIntCreateRectRgnIndirect(&Window->WindowRect); NtGdiOffsetRgn(hRgnWindow, -Window->WindowRect.left, @@ -225,12 +225,11 @@ IntInvalidateWindows(PWINDOW_OBJECT Window, HRGN hRgn, ULONG Flags) RgnType = NtGdiCombineRgn(hRgn, hRgn, hRgnWindow, RGN_AND); NtGdiDeleteObject(hRgnWindow); } -#ifdef TODO else { - RgnType = NtGdiCombineRgn(hRgn, hRgn, Window->WindowRegion, RGN_AND); + RgnType = NtGdiCombineRgn(hRgn, hRgn, Window->WindowRegion, RGN_AND); + IntUnLockWindowUpdate(Window); } -#endif /* * Save current state of pending updates diff --git a/reactos/subsys/win32k/ntuser/winpos.c b/reactos/subsys/win32k/ntuser/winpos.c index 041b87de0e3..c120edda7f9 100644 --- a/reactos/subsys/win32k/ntuser/winpos.c +++ b/reactos/subsys/win32k/ntuser/winpos.c @@ -16,7 +16,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ -/* $Id: winpos.c,v 1.101 2004/02/26 22:23:55 weiden Exp $ +/* $Id: winpos.c,v 1.102 2004/03/22 20:14:29 weiden Exp $ * * COPYRIGHT: See COPYING in the top level directory * PROJECT: ReactOS kernel @@ -774,6 +774,7 @@ WinPosSetWindowPos(HWND Wnd, HWND WndInsertAfter, INT x, INT y, INT cx, WINDOWPOS WinPos; RECT NewWindowRect; RECT NewClientRect; + PROSRGNDATA VisRgn; HRGN VisBefore = NULL; HRGN VisAfter = NULL; HRGN DirtyRgn = NULL; @@ -845,13 +846,19 @@ WinPosSetWindowPos(HWND Wnd, HWND WndInsertAfter, INT x, INT y, INT cx, (SWP_NOMOVE | SWP_NOSIZE | SWP_NOZORDER)) { VisBefore = VIS_ComputeVisibleRegion(Window, FALSE, FALSE, TRUE); + VisRgn = NULL; - if (VisBefore != NULL && - UnsafeIntGetRgnBox(VisBefore, &TempRect) == NULLREGION) + if (VisBefore != NULL && (VisRgn = (PROSRGNDATA)RGNDATA_LockRgn(VisBefore)) && + UnsafeIntGetRgnBox(VisRgn, &TempRect) == NULLREGION) { + RGNDATA_UnlockRgn(VisBefore); NtGdiDeleteObject(VisBefore); VisBefore = NULL; } + else if(VisRgn) + { + RGNDATA_UnlockRgn(VisBefore); + } } WvrFlags = WinPosDoNCCALCSize(Window, &WinPos, &NewWindowRect, &NewClientRect); @@ -940,13 +947,19 @@ WinPosSetWindowPos(HWND Wnd, HWND WndInsertAfter, INT x, INT y, INT cx, /* Determine the new visible region */ VisAfter = VIS_ComputeVisibleRegion(Window, FALSE, FALSE, TRUE); + VisRgn = NULL; - if (VisAfter != NULL && - UnsafeIntGetRgnBox(VisAfter, &TempRect) == NULLREGION) + if (VisAfter != NULL && (VisRgn = (PROSRGNDATA)RGNDATA_LockRgn(VisAfter)) && + UnsafeIntGetRgnBox(VisRgn, &TempRect) == NULLREGION) { + RGNDATA_UnlockRgn(VisAfter); NtGdiDeleteObject(VisAfter); VisAfter = NULL; } + else if(VisRgn) + { + RGNDATA_UnlockRgn(VisAfter); + } /* * Determine which pixels can be copied from the old window position @@ -995,15 +1008,22 @@ WinPosSetWindowPos(HWND Wnd, HWND WndInsertAfter, INT x, INT y, INT cx, * there's nothing to copy. Also, it's no use copying bits onto * themselves. */ - if (UnsafeIntGetRgnBox(CopyRgn, &CopyRect) == NULLREGION) + VisRgn = NULL; + if ((VisRgn = (PROSRGNDATA)RGNDATA_LockRgn(CopyRgn)) && + UnsafeIntGetRgnBox(VisRgn, &CopyRect) == NULLREGION) { /* Nothing to copy, clean up */ + RGNDATA_UnlockRgn(CopyRgn); NtGdiDeleteObject(CopyRgn); CopyRgn = NULL; } else if (OldWindowRect.left != NewWindowRect.left || OldWindowRect.top != NewWindowRect.top) { + if(VisRgn) + { + RGNDATA_UnlockRgn(CopyRgn); + } /* * Small trick here: there is no function to bitblt a region. So * we set the region as the clipping region, take the bounding box @@ -1026,6 +1046,10 @@ WinPosSetWindowPos(HWND Wnd, HWND WndInsertAfter, INT x, INT y, INT cx, NtUserReleaseDC(Wnd, Dc); IntValidateParent(Window, CopyRgn); } + else if(VisRgn) + { + RGNDATA_UnlockRgn(CopyRgn); + } } else { diff --git a/reactos/subsys/win32k/objects/cliprgn.c b/reactos/subsys/win32k/objects/cliprgn.c index 7be727cc1da..d7a6579f4c8 100644 --- a/reactos/subsys/win32k/objects/cliprgn.c +++ b/reactos/subsys/win32k/objects/cliprgn.c @@ -16,7 +16,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ -/* $Id: cliprgn.c,v 1.29 2003/12/21 18:38:37 navaraf Exp $ */ +/* $Id: cliprgn.c,v 1.30 2004/03/22 20:14:29 weiden Exp $ */ #undef WIN32_LEAN_AND_MEAN #include @@ -162,12 +162,19 @@ int FASTCALL IntGdiGetClipBox(HDC hDC, LPRECT rc) { + PROSRGNDATA Rgn; int retval; PDC dc; if (!(dc = DC_LockDc(hDC))) return ERROR; - retval = UnsafeIntGetRgnBox(dc->w.hGCClipRgn, rc); + if(!(Rgn = RGNDATA_LockRgn(dc->w.hGCClipRgn))) + { + DC_UnlockDc( hDC ); + return ERROR; + } + retval = UnsafeIntGetRgnBox(Rgn, rc); + RGNDATA_UnlockRgn(dc->w.hGCClipRgn); IntDPtoLP(dc, (LPPOINT)rc, 2); DC_UnlockDc( hDC ); return(retval); @@ -260,6 +267,8 @@ BOOL STDCALL NtGdiPtVisible(HDC hDC, BOOL STDCALL NtGdiRectVisible(HDC hDC, CONST PRECT UnsafeRect) { + NTSTATUS Status; + PROSRGNDATA Rgn; PDC dc = DC_LockDc(hDC); BOOL Result = FALSE; RECT Rect; @@ -269,12 +278,21 @@ BOOL STDCALL NtGdiRectVisible(HDC hDC, return FALSE; } - MmCopyFromCaller(&Rect, UnsafeRect, sizeof(RECT)); + Status = MmCopyFromCaller(&Rect, UnsafeRect, sizeof(RECT)); + if(!NT_SUCCESS(Status)) + { + DC_UnlockDc(hDC); + return FALSE; + } if (dc->w.hGCClipRgn) { - IntLPtoDP(dc, (LPPOINT)&Rect, 2); - Result = UnsafeIntRectInRegion(dc->w.hGCClipRgn, &Rect); + if((Rgn = (PROSRGNDATA)RGNDATA_LockRgn(dc->w.hGCClipRgn))) + { + IntLPtoDP(dc, (LPPOINT)&Rect, 2); + Result = UnsafeIntRectInRegion(Rgn, &Rect); + RGNDATA_UnlockRgn(dc->w.hGCClipRgn); + } } DC_UnlockDc(hDC); diff --git a/reactos/subsys/win32k/objects/region.c b/reactos/subsys/win32k/objects/region.c index 3ae533bc2d3..6e3fc35b901 100644 --- a/reactos/subsys/win32k/objects/region.c +++ b/reactos/subsys/win32k/objects/region.c @@ -16,7 +16,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ -/* $Id: region.c,v 1.42 2004/02/19 21:12:10 weiden Exp $ */ +/* $Id: region.c,v 1.43 2004/03/22 20:14:29 weiden Exp $ */ #undef WIN32_LEAN_AND_MEAN #include #include @@ -1674,7 +1674,7 @@ NtGdiCreateRectRgnIndirect(CONST PRECT rc) return(UnsafeIntCreateRectRgnIndirect(&SafeRc)); } -HRGN STDCALL +HRGN FASTCALL UnsafeIntCreateRectRgnIndirect(CONST PRECT rc) { return(NtGdiCreateRectRgn(rc->left, rc->top, rc->right, rc->bottom)); @@ -1735,10 +1735,10 @@ NtGdiCreateRoundRectRgn(INT left, INT top, INT right, INT bottom, /* move toward center */ rect.top = top++; rect.bottom = rect.top + 1; - UnsafeIntUnionRectWithRgn( hrgn, &rect ); + UnsafeIntUnionRectWithRgn( obj, &rect ); rect.top = --bottom; rect.bottom = rect.top + 1; - UnsafeIntUnionRectWithRgn( hrgn, &rect ); + UnsafeIntUnionRectWithRgn( obj, &rect ); yd -= 2*asq; d -= yd; } @@ -1747,7 +1747,6 @@ NtGdiCreateRoundRectRgn(INT left, INT top, INT right, INT bottom, xd += 2*bsq; d += bsq + xd; } - /* Loop to draw second half of quadrant */ d += (3 * (asq-bsq) / 2 - (xd+yd)) / 2; @@ -1756,10 +1755,10 @@ NtGdiCreateRoundRectRgn(INT left, INT top, INT right, INT bottom, /* next vertical point */ rect.top = top++; rect.bottom = rect.top + 1; - UnsafeIntUnionRectWithRgn( hrgn, &rect ); + UnsafeIntUnionRectWithRgn( obj, &rect ); rect.top = --bottom; rect.bottom = rect.top + 1; - UnsafeIntUnionRectWithRgn( hrgn, &rect ); + UnsafeIntUnionRectWithRgn( obj, &rect ); if (d < 0) /* if nearest pixel is outside ellipse */ { rect.left--; /* move away from center */ @@ -1770,14 +1769,13 @@ NtGdiCreateRoundRectRgn(INT left, INT top, INT right, INT bottom, yd -= 2*asq; d += asq - yd; } - /* Add the inside rectangle */ if (top <= bottom) { rect.top = top; rect.bottom = bottom; - UnsafeIntUnionRectWithRgn( hrgn, &rect ); + UnsafeIntUnionRectWithRgn( obj, &rect ); } RGNDATA_UnlockRgn( hrgn ); return hrgn; @@ -1888,18 +1886,16 @@ NtGdiFrameRgn(HDC hDC, UNIMPLEMENTED; } -INT STDCALL -UnsafeIntGetRgnBox(HRGN hRgn, +INT FASTCALL +UnsafeIntGetRgnBox(PROSRGNDATA Rgn, LPRECT pRect) { - PROSRGNDATA rgn = RGNDATA_LockRgn(hRgn); DWORD ret; - if (rgn) + if (Rgn) { - *pRect = rgn->rdh.rcBound; - ret = rgn->rdh.iType; - RGNDATA_UnlockRgn( hRgn ); + *pRect = Rgn->rdh.rcBound; + ret = Rgn->rdh.iType; return ret; } @@ -1911,10 +1907,17 @@ INT STDCALL NtGdiGetRgnBox(HRGN hRgn, LPRECT pRect) { + PROSRGNDATA Rgn; RECT SafeRect; DWORD ret; - ret = UnsafeIntGetRgnBox(hRgn, &SafeRect); + if (!(Rgn = RGNDATA_LockRgn(hRgn))) + { + return ERROR; + } + + ret = UnsafeIntGetRgnBox(Rgn, &SafeRect); + RGNDATA_UnlockRgn(hRgn); if (ERROR == ret) { return ret; @@ -2066,31 +2069,25 @@ NtGdiPtInRegion(HRGN hRgn, BOOL FASTCALL -UnsafeIntRectInRegion(HRGN hRgn, +UnsafeIntRectInRegion(PROSRGNDATA Rgn, CONST LPRECT rc) { - PROSRGNDATA rgn; PRECT pCurRect, pRectEnd; - BOOL bRet = FALSE; - - if( !( rgn = RGNDATA_LockRgn(hRgn) ) ) - return ERROR; // this is (just) a useful optimization - if((rgn->rdh.nCount > 0) && EXTENTCHECK(&rgn->rdh.rcBound, rc)) + if((Rgn->rdh.nCount > 0) && EXTENTCHECK(&Rgn->rdh.rcBound, rc)) { - for (pCurRect = (PRECT)rgn->Buffer, pRectEnd = pCurRect + rgn->rdh.nCount; pCurRect < pRectEnd; pCurRect++) + for (pCurRect = (PRECT)Rgn->Buffer, pRectEnd = pCurRect + Rgn->rdh.nCount; pCurRect < pRectEnd; pCurRect++) { if (pCurRect->bottom <= rc->top) continue; // not far enough down yet if (pCurRect->top >= rc->bottom) break; // too far down if (pCurRect->right <= rc->left) continue; // not far enough over yet if (pCurRect->left >= rc->right) continue; - bRet = TRUE; - break; + + return TRUE; } } - RGNDATA_UnlockRgn(hRgn); - return bRet; + return FALSE; } BOOL @@ -2098,15 +2095,25 @@ STDCALL NtGdiRectInRegion(HRGN hRgn, CONST LPRECT unsaferc) { + PROSRGNDATA Rgn; RECT rc; + BOOL Ret; + + if(!(Rgn = RGNDATA_LockRgn(hRgn))) + { + return ERROR; + } if (!NT_SUCCESS(MmCopyFromCaller(&rc, unsaferc, sizeof(RECT)))) { + RGNDATA_UnlockRgn(hRgn); DPRINT1("NtGdiRectInRegion: bogus rc\n"); return ERROR; } - - return UnsafeIntRectInRegion(hRgn, &rc); + + Ret = UnsafeIntRectInRegion(Rgn, &rc); + RGNDATA_UnlockRgn(hRgn); + return Ret; } BOOL @@ -2145,36 +2152,34 @@ NtGdiSetRectRgn(HRGN hRgn, return TRUE; } -HRGN FASTCALL -UnsafeIntUnionRectWithRgn(HRGN hDest, CONST PRECT Rect) +VOID FASTCALL +UnsafeIntUnionRectWithRgn(PROSRGNDATA RgnDest, CONST PRECT Rect) { - PROSRGNDATA pRgn; - - pRgn = RGNDATA_LockRgn(hDest); - if (NULL == pRgn) - { - SetLastWin32Error(ERROR_INVALID_HANDLE); - return NULL; - } - - REGION_UnionRectWithRegion(Rect, pRgn); - RGNDATA_UnlockRgn(hDest); - - return hDest; + REGION_UnionRectWithRegion(Rect, RgnDest); } HRGN STDCALL NtGdiUnionRectWithRgn(HRGN hDest, CONST PRECT UnsafeRect) { RECT SafeRect; + PROSRGNDATA Rgn; + + if(!(Rgn = (PROSRGNDATA)RGNDATA_UnlockRgn(hDest))) + { + SetLastWin32Error(ERROR_INVALID_HANDLE); + return NULL; + } if (! NT_SUCCESS(MmCopyFromCaller(&SafeRect, UnsafeRect, sizeof(RECT)))) { + RGNDATA_UnlockRgn(hDest); SetLastWin32Error(ERROR_INVALID_PARAMETER); return NULL; } - - return UnsafeIntUnionRectWithRgn(hDest, &SafeRect); + + UnsafeIntUnionRectWithRgn(Rgn, &SafeRect); + RGNDATA_UnlockRgn(hDest); + return hDest; } /*!