From 2e019b2fd1cea30a0839b69a6f071b3a65c4fa9c Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 23 Oct 2018 11:52:06 +0200 Subject: [PATCH] Implemented GFX locking and enforce return value checks. To fix #4825 GFX functions must now aquire a lock before accessing surfaces. This prevents simultaneous update of internal data by client and gfx threads. Also enforce return value checks, where not already done. --- client/X11/xf_gfx.c | 23 ++++- include/freerdp/client/rdpgfx.h | 5 +- include/freerdp/gdi/gfx.h | 4 +- libfreerdp/gdi/gfx.c | 211 ++++++++++++++++++++++++++++++---------- 4 files changed, 183 insertions(+), 60 deletions(-) diff --git a/client/X11/xf_gfx.c b/client/X11/xf_gfx.c index b68a297..d863173 100644 --- a/client/X11/xf_gfx.c +++ b/client/X11/xf_gfx.c @@ -129,6 +129,7 @@ static UINT xf_UpdateSurfaces(RdpgfxClientContext* context) if (gdi->suppressOutput) return CHANNEL_RC_OK; + EnterCriticalSection(&context->mux); context->GetSurfaceIds(context, &pSurfaceIds, &count); for (index = 0; index < count; index++) @@ -145,6 +146,7 @@ static UINT xf_UpdateSurfaces(RdpgfxClientContext* context) } free(pSurfaceIds); + LeaveCriticalSection(&context->mux); return status; } @@ -153,18 +155,22 @@ UINT xf_OutputExpose(xfContext* xfc, UINT32 x, UINT32 y, { UINT16 count; UINT32 index; - UINT status = CHANNEL_RC_OK; + UINT status = ERROR_INTERNAL_ERROR; xfGfxSurface* surface; RECTANGLE_16 invalidRect; RECTANGLE_16 surfaceRect; RECTANGLE_16 intersection; UINT16* pSurfaceIds = NULL; RdpgfxClientContext* context = xfc->context.gdi->gfx; + EnterCriticalSection(&context->mux); invalidRect.left = x; invalidRect.top = y; invalidRect.right = x + width; invalidRect.bottom = y + height; - context->GetSurfaceIds(context, &pSurfaceIds, &count); + status = context->GetSurfaceIds(context, &pSurfaceIds, &count); + + if (status != CHANNEL_RC_OK) + goto fail; for (index = 0; index < count; index++) { @@ -193,6 +199,12 @@ UINT xf_OutputExpose(xfContext* xfc, UINT32 x, UINT32 y, free(pSurfaceIds); IFCALLRET(context->UpdateSurfaces, status, context); + + if (status != CHANNEL_RC_OK) + goto fail; + +fail: + LeaveCriticalSection(&context->mux); return status; } @@ -343,6 +355,8 @@ static UINT xf_DeleteSurface(RdpgfxClientContext* context, { rdpCodecs* codecs = NULL; xfGfxSurface* surface = NULL; + UINT status; + EnterCriticalSection(&context->mux); surface = (xfGfxSurface*) context->GetSurfaceData(context, deleteSurface->surfaceId); @@ -360,13 +374,14 @@ static UINT xf_DeleteSurface(RdpgfxClientContext* context, free(surface); } - context->SetSurfaceData(context, deleteSurface->surfaceId, NULL); + status = context->SetSurfaceData(context, deleteSurface->surfaceId, NULL); if (codecs && codecs->progressive) progressive_delete_surface_context(codecs->progressive, deleteSurface->surfaceId); - return CHANNEL_RC_OK; + LeaveCriticalSection(&context->mux); + return status; } void xf_graphics_pipeline_init(xfContext* xfc, RdpgfxClientContext* gfx) diff --git a/include/freerdp/client/rdpgfx.h b/include/freerdp/client/rdpgfx.h index 161a2fa..f3f93ed 100644 --- a/include/freerdp/client/rdpgfx.h +++ b/include/freerdp/client/rdpgfx.h @@ -78,13 +78,14 @@ typedef void* (*pcRdpgfxGetCacheSlotData)(RdpgfxClientContext* context, typedef UINT(*pcRdpgfxUpdateSurfaces)(RdpgfxClientContext* context); typedef UINT(*pcRdpgfxUpdateSurfaceArea)(RdpgfxClientContext* context, UINT16 surfaceId, - UINT32 nrRects, const RECTANGLE_16* rects); + UINT32 nrRects, const RECTANGLE_16* rects); struct _rdpgfx_client_context { void* handle; void* custom; + /* Implementations require locking */ pcRdpgfxResetGraphics ResetGraphics; pcRdpgfxStartFrame StartFrame; pcRdpgfxEndFrame EndFrame; @@ -108,9 +109,11 @@ struct _rdpgfx_client_context pcRdpgfxSetCacheSlotData SetCacheSlotData; pcRdpgfxGetCacheSlotData GetCacheSlotData; + /* No locking required */ pcRdpgfxUpdateSurfaces UpdateSurfaces; pcRdpgfxUpdateSurfaceArea UpdateSurfaceArea; + CRITICAL_SECTION mux; PROFILER_DEFINE(SurfaceProfiler) }; diff --git a/include/freerdp/gdi/gfx.h b/include/freerdp/gdi/gfx.h index 24de3e0..48059d7 100644 --- a/include/freerdp/gdi/gfx.h +++ b/include/freerdp/gdi/gfx.h @@ -27,7 +27,7 @@ struct gdi_gfx_surface { UINT16 surfaceId; rdpCodecs* codecs; - H264_CONTEXT *h264; + H264_CONTEXT* h264; UINT32 width; UINT32 height; BYTE* data; @@ -55,7 +55,7 @@ typedef struct gdi_gfx_cache_entry gdiGfxCacheEntry; extern "C" { #endif -FREERDP_API void gdi_graphics_pipeline_init(rdpGdi* gdi, RdpgfxClientContext* gfx); +FREERDP_API BOOL gdi_graphics_pipeline_init(rdpGdi* gdi, RdpgfxClientContext* gfx); FREERDP_API void gdi_graphics_pipeline_uninit(rdpGdi* gdi, RdpgfxClientContext* gfx); #ifdef __cplusplus diff --git a/libfreerdp/gdi/gfx.c b/libfreerdp/gdi/gfx.c index fa3848c..087057f 100644 --- a/libfreerdp/gdi/gfx.c +++ b/libfreerdp/gdi/gfx.c @@ -49,6 +49,7 @@ static DWORD gfx_align_scanline(DWORD widthInBytes, DWORD alignment) static UINT gdi_ResetGraphics(RdpgfxClientContext* context, const RDPGFX_RESET_GRAPHICS_PDU* resetGraphics) { + UINT rc = ERROR_INTERNAL_ERROR; UINT32 index; UINT16 count; UINT32 DesktopWidth; @@ -58,6 +59,7 @@ static UINT gdi_ResetGraphics(RdpgfxClientContext* context, rdpGdi* gdi = (rdpGdi*) context->custom; rdpUpdate* update = gdi->context->update; rdpSettings* settings = gdi->context->settings; + EnterCriticalSection(&context->mux); DesktopWidth = resetGraphics->width; DesktopHeight = resetGraphics->height; @@ -86,10 +88,13 @@ static UINT gdi_ResetGraphics(RdpgfxClientContext* context, if (!freerdp_client_codecs_reset(gdi->context->codecs, FREERDP_CODEC_ALL, gdi->width, gdi->height)) - return ERROR_INTERNAL_ERROR; + goto fail; gdi->graphicsReset = TRUE; - return CHANNEL_RC_OK; + rc = CHANNEL_RC_OK; +fail: + LeaveCriticalSection(&context->mux); + return rc; } static UINT gdi_OutputUpdate(rdpGdi* gdi, gdiGfxSurface* surface) @@ -154,15 +159,16 @@ static UINT gdi_UpdateSurfaces(RdpgfxClientContext* context) { UINT16 count; UINT16 index; - UINT status = CHANNEL_RC_OK; + UINT status = ERROR_INTERNAL_ERROR; gdiGfxSurface* surface; UINT16* pSurfaceIds = NULL; rdpGdi* gdi = (rdpGdi*)context->custom; if (!gdi->graphicsReset) - return status; + return CHANNEL_RC_OK; context->GetSurfaceIds(context, &pSurfaceIds, &count); + status = CHANNEL_RC_OK; for (index = 0; index < count; index++) { @@ -241,7 +247,11 @@ static UINT gdi_SurfaceCommand_Uncompressed(rdpGdi* gdi, invalidRect.bottom = cmd->bottom; region16_union_rect(&(surface->invalidRegion), &(surface->invalidRegion), &invalidRect); - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, 1, &invalidRect); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, 1, + &invalidRect); + + if (status != CHANNEL_RC_OK) + goto fail; if (!gdi->inGfxFrame) { @@ -249,6 +259,7 @@ static UINT gdi_SurfaceCommand_Uncompressed(rdpGdi* gdi, IFCALLRET(context->UpdateSurfaces, status, context); } +fail: return status; } @@ -261,7 +272,7 @@ static UINT gdi_SurfaceCommand_RemoteFX(rdpGdi* gdi, RdpgfxClientContext* context, const RDPGFX_SURFACE_COMMAND* cmd) { - UINT status = CHANNEL_RC_OK; + UINT status = ERROR_INTERNAL_ERROR; gdiGfxSurface* surface; REGION16 invalidRegion; const RECTANGLE_16* rects; @@ -284,24 +295,27 @@ static UINT gdi_SurfaceCommand_RemoteFX(rdpGdi* gdi, surface->height, &invalidRegion)) { WLog_ERR(TAG, "Failed to process RemoteFX message"); - region16_uninit(&invalidRegion); - return ERROR_INTERNAL_ERROR; + goto fail; } rects = region16_rects(&invalidRegion, &nrRects); - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, nrRects, rects); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, + nrRects, rects); + + if (status != CHANNEL_RC_OK) + goto fail; for (x = 0; x < nrRects; x++) region16_union_rect(&surface->invalidRegion, &surface->invalidRegion, &rects[x]); - region16_uninit(&invalidRegion); - if (!gdi->inGfxFrame) { status = CHANNEL_RC_NOT_INITIALIZED; IFCALLRET(context->UpdateSurfaces, status, context); } +fail: + region16_uninit(&invalidRegion); return status; } @@ -345,7 +359,11 @@ static UINT gdi_SurfaceCommand_ClearCodec(rdpGdi* gdi, invalidRect.bottom = cmd->bottom; region16_union_rect(&(surface->invalidRegion), &(surface->invalidRegion), &invalidRect); - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, 1, &invalidRect); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, 1, + &invalidRect); + + if (status != CHANNEL_RC_OK) + goto fail; if (!gdi->inGfxFrame) { @@ -353,6 +371,7 @@ static UINT gdi_SurfaceCommand_ClearCodec(rdpGdi* gdi, IFCALLRET(context->UpdateSurfaces, status, context); } +fail: return status; } @@ -392,7 +411,11 @@ static UINT gdi_SurfaceCommand_Planar(rdpGdi* gdi, RdpgfxClientContext* context, invalidRect.bottom = cmd->bottom; region16_union_rect(&(surface->invalidRegion), &(surface->invalidRegion), &invalidRect); - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, 1, &invalidRect); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, 1, + &invalidRect); + + if (status != CHANNEL_RC_OK) + goto fail; if (!gdi->inGfxFrame) { @@ -400,6 +423,7 @@ static UINT gdi_SurfaceCommand_Planar(rdpGdi* gdi, RdpgfxClientContext* context, IFCALLRET(context->UpdateSurfaces, status, context); } +fail: return status; } @@ -465,8 +489,11 @@ static UINT gdi_SurfaceCommand_AVC420(rdpGdi* gdi, (RECTANGLE_16*) & (meta->regionRects[i])); } - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, - meta->numRegionRects, meta->regionRects); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, + meta->numRegionRects, meta->regionRects); + + if (status != CHANNEL_RC_OK) + goto fail; if (!gdi->inGfxFrame) { @@ -474,6 +501,7 @@ static UINT gdi_SurfaceCommand_AVC420(rdpGdi* gdi, IFCALLRET(context->UpdateSurfaces, status, context); } +fail: return status; #else return ERROR_NOT_SUPPORTED; @@ -551,16 +579,19 @@ static UINT gdi_SurfaceCommand_AVC444(rdpGdi* gdi, RdpgfxClientContext* context, region16_union_rect(&(surface->invalidRegion), &(surface->invalidRegion), &(meta1->regionRects[i])); } - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, - meta1->numRegionRects, meta1->regionRects); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, + meta1->numRegionRects, meta1->regionRects); for (i = 0; i < meta2->numRegionRects; i++) { region16_union_rect(&(surface->invalidRegion), &(surface->invalidRegion), &(meta2->regionRects[i])); } - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, - meta2->numRegionRects, meta2->regionRects); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, + meta2->numRegionRects, meta2->regionRects); + + if (status != CHANNEL_RC_OK) + goto fail; if (!gdi->inGfxFrame) { @@ -568,6 +599,7 @@ static UINT gdi_SurfaceCommand_AVC444(rdpGdi* gdi, RdpgfxClientContext* context, IFCALLRET(context->UpdateSurfaces, status, context); } +fail: free(regionRects); return status; #else @@ -610,7 +642,11 @@ static UINT gdi_SurfaceCommand_Alpha(rdpGdi* gdi, RdpgfxClientContext* context, invalidRect.bottom = cmd->bottom; region16_union_rect(&(surface->invalidRegion), &(surface->invalidRegion), &invalidRect); - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, 1, &invalidRect); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, 1, + &invalidRect); + + if (status != CHANNEL_RC_OK) + goto fail; if (!gdi->inGfxFrame) { @@ -618,6 +654,7 @@ static UINT gdi_SurfaceCommand_Alpha(rdpGdi* gdi, RdpgfxClientContext* context, IFCALLRET(context->UpdateSurfaces, status, context); } +fail: return status; } @@ -674,7 +711,11 @@ static UINT gdi_SurfaceCommand_Progressive(rdpGdi* gdi, } rects = region16_rects(&invalidRegion, &nrRects); - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, nrRects, rects); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, + nrRects, rects); + + if (status != CHANNEL_RC_OK) + goto fail; for (x = 0; x < nrRects; x++) region16_union_rect(&surface->invalidRegion, &surface->invalidRegion, &rects[x]); @@ -687,6 +728,7 @@ static UINT gdi_SurfaceCommand_Progressive(rdpGdi* gdi, IFCALLRET(context->UpdateSurfaces, status, context); } +fail: return status; } @@ -704,6 +746,7 @@ static UINT gdi_SurfaceCommand(RdpgfxClientContext* context, if (!context || !cmd) return ERROR_INVALID_PARAMETER; + EnterCriticalSection(&context->mux); WLog_Print(gdi->log, WLOG_TRACE, "surfaceId=%"PRIu32", codec=%"PRIu32", contextId=%"PRIu32", format=%s, " "left=%"PRIu32", top=%"PRIu32", right=%"PRIu32", bottom=%"PRIu32", width=%"PRIu32", height=%"PRIu32" " @@ -756,6 +799,7 @@ static UINT gdi_SurfaceCommand(RdpgfxClientContext* context, break; } + LeaveCriticalSection(&context->mux); return status; } @@ -778,19 +822,21 @@ static UINT gdi_DeleteEncodingContext(RdpgfxClientContext* context, static UINT gdi_CreateSurface(RdpgfxClientContext* context, const RDPGFX_CREATE_SURFACE_PDU* createSurface) { + UINT rc = ERROR_INTERNAL_ERROR; gdiGfxSurface* surface; rdpGdi* gdi = (rdpGdi*) context->custom; + EnterCriticalSection(&context->mux); surface = (gdiGfxSurface*) calloc(1, sizeof(gdiGfxSurface)); if (!surface) - return ERROR_INTERNAL_ERROR; + goto fail; surface->codecs = gdi->context->codecs; if (!surface->codecs) { free(surface); - return CHANNEL_RC_NO_MEMORY; + goto fail; } surface->surfaceId = createSurface->surfaceId; @@ -818,13 +864,15 @@ static UINT gdi_CreateSurface(RdpgfxClientContext* context, if (!surface->data) { free(surface); - return ERROR_INTERNAL_ERROR; + goto fail; } surface->outputMapped = FALSE; region16_init(&surface->invalidRegion); - context->SetSurfaceData(context, surface->surfaceId, (void*) surface); - return CHANNEL_RC_OK; + rc = context->SetSurfaceData(context, surface->surfaceId, (void*) surface); +fail: + LeaveCriticalSection(&context->mux); + return rc; } /** @@ -835,8 +883,10 @@ static UINT gdi_CreateSurface(RdpgfxClientContext* context, static UINT gdi_DeleteSurface(RdpgfxClientContext* context, const RDPGFX_DELETE_SURFACE_PDU* deleteSurface) { + UINT rc = ERROR_INTERNAL_ERROR; rdpCodecs* codecs = NULL; gdiGfxSurface* surface = NULL; + EnterCriticalSection(&context->mux); surface = (gdiGfxSurface*) context->GetSurfaceData(context, deleteSurface->surfaceId); if (surface) @@ -850,12 +900,13 @@ static UINT gdi_DeleteSurface(RdpgfxClientContext* context, free(surface); } - context->SetSurfaceData(context, deleteSurface->surfaceId, NULL); + rc = context->SetSurfaceData(context, deleteSurface->surfaceId, NULL); if (codecs && codecs->progressive) progressive_delete_surface_context(codecs->progressive, deleteSurface->surfaceId); - return CHANNEL_RC_OK; + LeaveCriticalSection(&context->mux); + return rc; } /** @@ -866,7 +917,7 @@ static UINT gdi_DeleteSurface(RdpgfxClientContext* context, static UINT gdi_SolidFill(RdpgfxClientContext* context, const RDPGFX_SOLID_FILL_PDU* solidFill) { - UINT status = CHANNEL_RC_OK; + UINT status = ERROR_INTERNAL_ERROR; UINT16 index; UINT32 color; BYTE a, r, g, b; @@ -875,11 +926,12 @@ static UINT gdi_SolidFill(RdpgfxClientContext* context, gdiGfxSurface* surface; RECTANGLE_16 invalidRect; rdpGdi* gdi = (rdpGdi*) context->custom; + EnterCriticalSection(&context->mux); surface = (gdiGfxSurface*) context->GetSurfaceData(context, solidFill->surfaceId); if (!surface) - return ERROR_INTERNAL_ERROR; + goto fail; b = solidFill->fillPixel.B; g = solidFill->fillPixel.G; @@ -901,14 +953,17 @@ static UINT gdi_SolidFill(RdpgfxClientContext* context, if (!freerdp_image_fill(surface->data, surface->format, surface->scanline, rect->left, rect->top, nWidth, nHeight, color)) - return ERROR_INTERNAL_ERROR; + goto fail; region16_union_rect(&(surface->invalidRegion), &(surface->invalidRegion), &invalidRect); } - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, - solidFill->fillRectCount, solidFill->fillRects); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, + solidFill->fillRectCount, solidFill->fillRects); + + if (status != CHANNEL_RC_OK) + goto fail; if (!gdi->inGfxFrame) { @@ -916,6 +971,12 @@ static UINT gdi_SolidFill(RdpgfxClientContext* context, IFCALLRET(context->UpdateSurfaces, status, context); } + if (status != CHANNEL_RC_OK) + goto fail; + + status = CHANNEL_RC_OK; +fail: + LeaveCriticalSection(&context->mux); return status; } @@ -927,7 +988,7 @@ static UINT gdi_SolidFill(RdpgfxClientContext* context, static UINT gdi_SurfaceToSurface(RdpgfxClientContext* context, const RDPGFX_SURFACE_TO_SURFACE_PDU* surfaceToSurface) { - UINT status = CHANNEL_RC_OK; + UINT status = ERROR_INTERNAL_ERROR; UINT16 index; BOOL sameSurface; UINT32 nWidth, nHeight; @@ -937,6 +998,7 @@ static UINT gdi_SurfaceToSurface(RdpgfxClientContext* context, gdiGfxSurface* surfaceSrc; gdiGfxSurface* surfaceDst; rdpGdi* gdi = (rdpGdi*) context->custom; + EnterCriticalSection(&context->mux); rectSrc = &(surfaceToSurface->rectSrc); surfaceSrc = (gdiGfxSurface*) context->GetSurfaceData(context, surfaceToSurface->surfaceIdSrc); @@ -950,7 +1012,7 @@ static UINT gdi_SurfaceToSurface(RdpgfxClientContext* context, surfaceDst = surfaceSrc; if (!surfaceSrc || !surfaceDst) - return ERROR_INTERNAL_ERROR; + goto fail; nWidth = rectSrc->right - rectSrc->left; nHeight = rectSrc->bottom - rectSrc->top; @@ -965,7 +1027,7 @@ static UINT gdi_SurfaceToSurface(RdpgfxClientContext* context, surfaceSrc->data, surfaceSrc->format, surfaceSrc->scanline, rectSrc->left, rectSrc->top, NULL, FREERDP_FLIP_NONE)) - return ERROR_INTERNAL_ERROR; + goto fail; invalidRect.left = destPt->x; invalidRect.top = destPt->y; @@ -973,15 +1035,25 @@ static UINT gdi_SurfaceToSurface(RdpgfxClientContext* context, invalidRect.bottom = destPt->y + rectSrc->bottom; region16_union_rect(&surfaceDst->invalidRegion, &surfaceDst->invalidRegion, &invalidRect); - IFCALL(context->UpdateSurfaceArea, context, surfaceDst->surfaceId, 1, &invalidRect); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surfaceDst->surfaceId, 1, + &invalidRect); + + if (status != CHANNEL_RC_OK) + goto fail; } if (!gdi->inGfxFrame) { status = CHANNEL_RC_NOT_INITIALIZED; IFCALLRET(context->UpdateSurfaces, status, context); + + if (status != CHANNEL_RC_OK) + goto fail; } + status = CHANNEL_RC_OK; +fail: + LeaveCriticalSection(&context->mux); return status; } @@ -996,16 +1068,18 @@ static UINT gdi_SurfaceToCache(RdpgfxClientContext* context, const RECTANGLE_16* rect; gdiGfxSurface* surface; gdiGfxCacheEntry* cacheEntry; + UINT rc = ERROR_INTERNAL_ERROR; + EnterCriticalSection(&context->mux); rect = &(surfaceToCache->rectSrc); surface = (gdiGfxSurface*) context->GetSurfaceData(context, surfaceToCache->surfaceId); if (!surface) - return ERROR_INTERNAL_ERROR; + goto fail; cacheEntry = (gdiGfxCacheEntry*) calloc(1, sizeof(gdiGfxCacheEntry)); if (!cacheEntry) - return ERROR_NOT_ENOUGH_MEMORY; + goto fail; cacheEntry->width = (UINT32)(rect->right - rect->left); cacheEntry->height = (UINT32)(rect->bottom - rect->top); @@ -1016,7 +1090,7 @@ static UINT gdi_SurfaceToCache(RdpgfxClientContext* context, if (!cacheEntry->data) { free(cacheEntry); - return ERROR_NOT_ENOUGH_MEMORY; + goto fail; } if (!freerdp_image_copy(cacheEntry->data, cacheEntry->format, cacheEntry->scanline, @@ -1024,10 +1098,13 @@ static UINT gdi_SurfaceToCache(RdpgfxClientContext* context, surface->format, surface->scanline, rect->left, rect->top, NULL, FREERDP_FLIP_NONE)) { free(cacheEntry); - return ERROR_INTERNAL_ERROR; + goto fail; } - return context->SetCacheSlotData(context, surfaceToCache->cacheSlot, (void*) cacheEntry); + rc = context->SetCacheSlotData(context, surfaceToCache->cacheSlot, (void*) cacheEntry); +fail: + LeaveCriticalSection(&context->mux); + return rc; } /** @@ -1038,18 +1115,19 @@ static UINT gdi_SurfaceToCache(RdpgfxClientContext* context, static UINT gdi_CacheToSurface(RdpgfxClientContext* context, const RDPGFX_CACHE_TO_SURFACE_PDU* cacheToSurface) { - UINT status = CHANNEL_RC_OK; + UINT status = ERROR_INTERNAL_ERROR; UINT16 index; RDPGFX_POINT16* destPt; gdiGfxSurface* surface; gdiGfxCacheEntry* cacheEntry; RECTANGLE_16 invalidRect; rdpGdi* gdi = (rdpGdi*) context->custom; + EnterCriticalSection(&context->mux); surface = (gdiGfxSurface*) context->GetSurfaceData(context, cacheToSurface->surfaceId); cacheEntry = (gdiGfxCacheEntry*) context->GetCacheSlotData(context, cacheToSurface->cacheSlot); if (!surface || !cacheEntry) - return ERROR_INTERNAL_ERROR; + goto fail; for (index = 0; index < cacheToSurface->destPtsCount; index++) { @@ -1059,7 +1137,7 @@ static UINT gdi_CacheToSurface(RdpgfxClientContext* context, destPt->x, destPt->y, cacheEntry->width, cacheEntry->height, cacheEntry->data, cacheEntry->format, cacheEntry->scanline, 0, 0, NULL, FREERDP_FLIP_NONE)) - return ERROR_INTERNAL_ERROR; + goto fail; invalidRect.left = destPt->x; invalidRect.top = destPt->y; @@ -1067,7 +1145,11 @@ static UINT gdi_CacheToSurface(RdpgfxClientContext* context, invalidRect.bottom = destPt->y + cacheEntry->height; region16_union_rect(&surface->invalidRegion, &surface->invalidRegion, &invalidRect); - IFCALL(context->UpdateSurfaceArea, context, surface->surfaceId, 1, &invalidRect); + status = IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaceArea, context, surface->surfaceId, 1, + &invalidRect); + + if (status != CHANNEL_RC_OK) + goto fail; } if (!gdi->inGfxFrame) @@ -1075,7 +1157,11 @@ static UINT gdi_CacheToSurface(RdpgfxClientContext* context, status = CHANNEL_RC_NOT_INITIALIZED; IFCALLRET(context->UpdateSurfaces, status, context); } + else + status = CHANNEL_RC_OK; +fail: + LeaveCriticalSection(&context->mux); return status; } @@ -1099,6 +1185,8 @@ static UINT gdi_EvictCacheEntry(RdpgfxClientContext* context, const RDPGFX_EVICT_CACHE_ENTRY_PDU* evictCacheEntry) { gdiGfxCacheEntry* cacheEntry; + UINT rc = ERROR_INTERNAL_ERROR; + EnterCriticalSection(&context->mux); cacheEntry = (gdiGfxCacheEntry*) context->GetCacheSlotData(context, evictCacheEntry->cacheSlot); @@ -1108,8 +1196,9 @@ static UINT gdi_EvictCacheEntry(RdpgfxClientContext* context, free(cacheEntry); } - context->SetCacheSlotData(context, evictCacheEntry->cacheSlot, NULL); - return CHANNEL_RC_OK; + rc = context->SetCacheSlotData(context, evictCacheEntry->cacheSlot, NULL); + LeaveCriticalSection(&context->mux); + return rc; } /** @@ -1120,18 +1209,23 @@ static UINT gdi_EvictCacheEntry(RdpgfxClientContext* context, static UINT gdi_MapSurfaceToOutput(RdpgfxClientContext* context, const RDPGFX_MAP_SURFACE_TO_OUTPUT_PDU* surfaceToOutput) { + UINT rc = ERROR_INTERNAL_ERROR; gdiGfxSurface* surface; + EnterCriticalSection(&context->mux); surface = (gdiGfxSurface*) context->GetSurfaceData(context, surfaceToOutput->surfaceId); if (!surface) - return ERROR_INTERNAL_ERROR; + goto fail; surface->outputMapped = TRUE; surface->outputOriginX = surfaceToOutput->outputOriginX; surface->outputOriginY = surfaceToOutput->outputOriginY; region16_clear(&surface->invalidRegion); - return CHANNEL_RC_OK; + rc = CHANNEL_RC_OK; +fail: + LeaveCriticalSection(&context->mux); + return rc; } /** @@ -1145,8 +1239,11 @@ static UINT gdi_MapSurfaceToWindow(RdpgfxClientContext* context, return CHANNEL_RC_OK; } -void gdi_graphics_pipeline_init(rdpGdi* gdi, RdpgfxClientContext* gfx) +BOOL gdi_graphics_pipeline_init(rdpGdi* gdi, RdpgfxClientContext* gfx) { + if (!gdi || !gfx) + return FALSE; + gdi->gfx = gfx; gfx->custom = (void*) gdi; gfx->ResetGraphics = gdi_ResetGraphics; @@ -1165,13 +1262,21 @@ void gdi_graphics_pipeline_init(rdpGdi* gdi, RdpgfxClientContext* gfx) gfx->MapSurfaceToOutput = gdi_MapSurfaceToOutput; gfx->MapSurfaceToWindow = gdi_MapSurfaceToWindow; gfx->UpdateSurfaces = gdi_UpdateSurfaces; - PROFILER_CREATE(gfx->SurfaceProfiler, "GFX-PROFILER") + InitializeCriticalSection(&gfx->mux); + PROFILER_CREATE(gfx->SurfaceProfiler, "GFX-PROFILER"); + return TRUE; } void gdi_graphics_pipeline_uninit(rdpGdi* gdi, RdpgfxClientContext* gfx) { - gdi->gfx = NULL; + if (gdi) + gdi->gfx = NULL; + + if (!gfx) + return; + gfx->custom = NULL; + DeleteCriticalSection(&gfx->mux); PROFILER_PRINT_HEADER PROFILER_PRINT(gfx->SurfaceProfiler) PROFILER_PRINT_FOOTER -- 2.7.4