wgl: Make the stw_framebuffer destructions threadsafe.

Ensure no other thread is accessing a framebuffer when it is being destroyed by
acquiring both the global and per-framebuffer mutexes. Normal access only
needs the global lock to walk the linked list and acquire the per-framebuffer
mutex.
This commit is contained in:
José Fonseca 2009-07-06 18:23:37 +01:00
parent 6f4167c8a2
commit 1068c15c61
5 changed files with 186 additions and 82 deletions

View File

@ -80,7 +80,7 @@ stw_copy_context(
struct stw_context *dst;
BOOL ret = FALSE;
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->ctx_mutex );
src = stw_lookup_context_locked( hglrcSrc );
dst = stw_lookup_context_locked( hglrcDst );
@ -93,7 +93,7 @@ stw_copy_context(
(void) mask;
}
pipe_mutex_unlock( stw_dev->mutex );
pipe_mutex_unlock( stw_dev->ctx_mutex );
return ret;
}
@ -107,7 +107,7 @@ stw_share_lists(
struct stw_context *ctx2;
BOOL ret = FALSE;
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->ctx_mutex );
ctx1 = stw_lookup_context_locked( hglrc1 );
ctx2 = stw_lookup_context_locked( hglrc2 );
@ -117,7 +117,7 @@ stw_share_lists(
ret = _mesa_share_state(ctx2->st->ctx, ctx1->st->ctx);
}
pipe_mutex_unlock( stw_dev->mutex );
pipe_mutex_unlock( stw_dev->ctx_mutex );
return ret;
}
@ -130,8 +130,10 @@ stw_viewport(GLcontext * glctx, GLint x, GLint y,
struct stw_framebuffer *fb;
fb = stw_framebuffer_from_hdc( ctx->hdc );
if(fb)
if(fb) {
stw_framebuffer_update(fb);
stw_framebuffer_release(fb);
}
}
UINT_PTR
@ -195,9 +197,9 @@ stw_create_layer_context(
ctx->st->ctx->DriverCtx = ctx;
ctx->st->ctx->Driver.Viewport = stw_viewport;
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->ctx_mutex );
ctx->hglrc = handle_table_add(stw_dev->ctx_table, ctx);
pipe_mutex_unlock( stw_dev->mutex );
pipe_mutex_unlock( stw_dev->ctx_mutex );
if (!ctx->hglrc)
goto no_hglrc;
@ -224,10 +226,10 @@ stw_delete_context(
if (!stw_dev)
return FALSE;
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->ctx_mutex );
ctx = stw_lookup_context_locked(hglrc);
handle_table_remove(stw_dev->ctx_table, hglrc);
pipe_mutex_unlock( stw_dev->mutex );
pipe_mutex_unlock( stw_dev->ctx_mutex );
if (ctx) {
struct stw_context *curctx = stw_current_context();
@ -254,9 +256,9 @@ stw_release_context(
if (!stw_dev)
return FALSE;
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->ctx_mutex );
ctx = stw_lookup_context_locked( hglrc );
pipe_mutex_unlock( stw_dev->mutex );
pipe_mutex_unlock( stw_dev->ctx_mutex );
if (!ctx)
return FALSE;
@ -304,9 +306,9 @@ stw_make_current(
HDC hdc,
UINT_PTR hglrc )
{
struct stw_context *curctx;
struct stw_context *ctx;
struct stw_framebuffer *fb;
struct stw_context *curctx = NULL;
struct stw_context *ctx = NULL;
struct stw_framebuffer *fb = NULL;
if (!stw_dev)
goto fail;
@ -328,13 +330,13 @@ stw_make_current(
return st_make_current( NULL, NULL, NULL );
}
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->ctx_mutex );
ctx = stw_lookup_context_locked( hglrc );
pipe_mutex_unlock( stw_dev->ctx_mutex );
if(!ctx)
goto fail;
fb = stw_framebuffer_from_hdc_locked( hdc );
fb = stw_framebuffer_from_hdc( hdc );
if(!fb) {
/* Applications should call SetPixelFormat before creating a context,
* but not all do, and the opengl32 runtime seems to use a default pixel
@ -342,13 +344,11 @@ stw_make_current(
*/
int iPixelFormat = GetPixelFormat(hdc);
if(iPixelFormat)
fb = stw_framebuffer_create_locked( hdc, iPixelFormat );
fb = stw_framebuffer_create( hdc, iPixelFormat );
if(!fb)
goto fail;
}
pipe_mutex_unlock( stw_dev->mutex );
if(fb->iPixelFormat != ctx->iPixelFormat)
goto fail;
@ -367,12 +367,16 @@ stw_make_current(
success:
assert(fb);
if(fb)
if(fb) {
stw_framebuffer_update(fb);
stw_framebuffer_release(fb);
}
return TRUE;
fail:
if(fb)
stw_framebuffer_release(fb);
st_make_current( NULL, NULL, NULL );
return FALSE;
}

View File

@ -69,8 +69,6 @@ stw_flush_frontbuffer(struct pipe_screen *screen,
fb = stw_framebuffer_from_hdc( hdc );
/* fb can be NULL if window was destroyed already */
if (fb) {
pipe_mutex_lock( fb->mutex );
#if DEBUG
{
struct pipe_surface *surface2;
@ -94,8 +92,7 @@ stw_flush_frontbuffer(struct pipe_screen *screen,
if(fb) {
stw_framebuffer_update(fb);
pipe_mutex_unlock( fb->mutex );
stw_framebuffer_release(fb);
}
}
@ -138,7 +135,8 @@ stw_init(const struct stw_winsys *stw_winsys)
stw_dev->screen->flush_frontbuffer = &stw_flush_frontbuffer;
pipe_mutex_init( stw_dev->mutex );
pipe_mutex_init( stw_dev->ctx_mutex );
pipe_mutex_init( stw_dev->fb_mutex );
stw_dev->ctx_table = handle_table_create();
if (!stw_dev->ctx_table) {
@ -179,7 +177,7 @@ stw_cleanup(void)
if (!stw_dev)
return;
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->ctx_mutex );
{
/* Ensure all contexts are destroyed */
i = handle_table_get_first_handle(stw_dev->ctx_table);
@ -189,11 +187,12 @@ stw_cleanup(void)
}
handle_table_destroy(stw_dev->ctx_table);
}
pipe_mutex_unlock( stw_dev->mutex );
pipe_mutex_unlock( stw_dev->ctx_mutex );
stw_framebuffer_cleanup();
pipe_mutex_destroy( stw_dev->mutex );
pipe_mutex_destroy( stw_dev->fb_mutex );
pipe_mutex_destroy( stw_dev->ctx_mutex );
stw_dev->screen->destroy(stw_dev->screen);

View File

@ -57,10 +57,10 @@ struct stw_device
unsigned pixelformat_count;
unsigned pixelformat_extended_count;
pipe_mutex mutex;
pipe_mutex ctx_mutex;
struct handle_table *ctx_table;
pipe_mutex fb_mutex;
struct stw_framebuffer *fb_head;
#ifdef DEBUG

View File

@ -45,6 +45,10 @@
#include "stw_tls.h"
/**
* Search the framebuffer with the matching HWND while holding the
* stw_dev::fb_mutex global lock.
*/
static INLINE struct stw_framebuffer *
stw_framebuffer_from_hwnd_locked(
HWND hwnd )
@ -52,13 +56,20 @@ stw_framebuffer_from_hwnd_locked(
struct stw_framebuffer *fb;
for (fb = stw_dev->fb_head; fb != NULL; fb = fb->next)
if (fb->hWnd == hwnd)
if (fb->hWnd == hwnd) {
pipe_mutex_lock(fb->mutex);
break;
}
return fb;
}
/**
* Destroy this framebuffer. Both stw_dev::fb_mutex and stw_framebuffer::mutex
* must be held, by this order. Obviously no further access to fb can be done
* after this.
*/
static INLINE void
stw_framebuffer_destroy_locked(
struct stw_framebuffer *fb )
@ -74,12 +85,23 @@ stw_framebuffer_destroy_locked(
st_unreference_framebuffer(fb->stfb);
pipe_mutex_unlock( fb->mutex );
pipe_mutex_destroy( fb->mutex );
FREE( fb );
}
void
stw_framebuffer_release(
struct stw_framebuffer *fb)
{
assert(fb);
pipe_mutex_unlock( fb->mutex );
}
static INLINE void
stw_framebuffer_get_size( struct stw_framebuffer *fb )
{
@ -134,38 +156,29 @@ stw_call_window_proc(
LPWINDOWPOS lpWindowPos = (LPWINDOWPOS)pParams->lParam;
if((lpWindowPos->flags & SWP_SHOWWINDOW) ||
!(lpWindowPos->flags & SWP_NOSIZE)) {
pipe_mutex_lock( stw_dev->mutex );
fb = stw_framebuffer_from_hwnd_locked( pParams->hwnd );
pipe_mutex_unlock( stw_dev->mutex );
fb = stw_framebuffer_from_hwnd( pParams->hwnd );
if(fb) {
pipe_mutex_lock( fb->mutex );
/* Size in WINDOWPOS includes the window frame, so get the size
* of the client area via GetClientRect. */
stw_framebuffer_get_size(fb);
pipe_mutex_unlock( fb->mutex );
stw_framebuffer_release(fb);
}
}
}
else if (pParams->message == WM_DESTROY) {
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->fb_mutex );
fb = stw_framebuffer_from_hwnd_locked( pParams->hwnd );
if(fb)
stw_framebuffer_destroy_locked(fb);
pipe_mutex_unlock( stw_dev->mutex );
pipe_mutex_unlock( stw_dev->fb_mutex );
}
return CallNextHookEx(tls_data->hCallWndProcHook, nCode, wParam, lParam);
}
/**
* Create a new framebuffer object which will correspond to the given HDC.
*/
struct stw_framebuffer *
stw_framebuffer_create_locked(
stw_framebuffer_create(
HDC hdc,
int iPixelFormat )
{
@ -194,8 +207,16 @@ stw_framebuffer_create_locked(
pipe_mutex_init( fb->mutex );
/* This is the only case where we lock the stw_framebuffer::mutex before
* stw_dev::fb_mutex, since no other thread can know about this framebuffer
* and we must prevent any other thread from destroying it before we return.
*/
pipe_mutex_lock( fb->mutex );
pipe_mutex_lock( stw_dev->fb_mutex );
fb->next = stw_dev->fb_head;
stw_dev->fb_head = fb;
pipe_mutex_unlock( stw_dev->fb_mutex );
return fb;
}
@ -205,8 +226,8 @@ BOOL
stw_framebuffer_allocate(
struct stw_framebuffer *fb)
{
pipe_mutex_lock( fb->mutex );
assert(fb);
if(!fb->stfb) {
const struct stw_pixelformat_info *pfi = fb->pfi;
enum pipe_format colorFormat, depthFormat, stencilFormat;
@ -242,8 +263,6 @@ stw_framebuffer_allocate(
fb->must_resize = TRUE;
}
pipe_mutex_unlock( fb->mutex );
return fb->stfb ? TRUE : FALSE;
}
@ -280,24 +299,27 @@ stw_framebuffer_cleanup( void )
struct stw_framebuffer *fb;
struct stw_framebuffer *next;
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->fb_mutex );
fb = stw_dev->fb_head;
while (fb) {
next = fb->next;
pipe_mutex_lock(fb->mutex);
stw_framebuffer_destroy_locked(fb);
fb = next;
}
stw_dev->fb_head = NULL;
pipe_mutex_unlock( stw_dev->mutex );
pipe_mutex_unlock( stw_dev->fb_mutex );
}
/**
* Given an hdc, return the corresponding stw_framebuffer.
*/
struct stw_framebuffer *
static INLINE struct stw_framebuffer *
stw_framebuffer_from_hdc_locked(
HDC hdc )
{
@ -314,8 +336,10 @@ stw_framebuffer_from_hdc_locked(
return stw_framebuffer_from_hwnd_locked(hwnd);
for (fb = stw_dev->fb_head; fb != NULL; fb = fb->next)
if (fb->hDC == hdc)
if (fb->hDC == hdc) {
pipe_mutex_lock(fb->mutex);
break;
}
return fb;
}
@ -330,9 +354,26 @@ stw_framebuffer_from_hdc(
{
struct stw_framebuffer *fb;
pipe_mutex_lock( stw_dev->mutex );
pipe_mutex_lock( stw_dev->fb_mutex );
fb = stw_framebuffer_from_hdc_locked(hdc);
pipe_mutex_unlock( stw_dev->mutex );
pipe_mutex_unlock( stw_dev->fb_mutex );
return fb;
}
/**
* Given an hdc, return the corresponding stw_framebuffer.
*/
struct stw_framebuffer *
stw_framebuffer_from_hwnd(
HWND hwnd )
{
struct stw_framebuffer *fb;
pipe_mutex_lock( stw_dev->fb_mutex );
fb = stw_framebuffer_from_hwnd_locked(hwnd);
pipe_mutex_unlock( stw_dev->fb_mutex );
return fb;
}
@ -352,22 +393,19 @@ stw_pixelformat_set(
if (index >= count)
return FALSE;
pipe_mutex_lock( stw_dev->mutex );
fb = stw_framebuffer_from_hdc_locked(hdc);
if(fb) {
/* SetPixelFormat must be called only once */
pipe_mutex_unlock( stw_dev->mutex );
stw_framebuffer_release( fb );
return FALSE;
}
fb = stw_framebuffer_create_locked(hdc, iPixelFormat);
fb = stw_framebuffer_create(hdc, iPixelFormat);
if(!fb) {
pipe_mutex_unlock( stw_dev->mutex );
return FALSE;
}
pipe_mutex_unlock( stw_dev->mutex );
stw_framebuffer_release( fb );
/* Some applications mistakenly use the undocumented wglSetPixelFormat
* function instead of SetPixelFormat, so we call SetPixelFormat here to
@ -384,13 +422,16 @@ int
stw_pixelformat_get(
HDC hdc )
{
int iPixelFormat = 0;
struct stw_framebuffer *fb;
fb = stw_framebuffer_from_hdc(hdc);
if(!fb)
return 0;
if(fb) {
iPixelFormat = fb->iPixelFormat;
stw_framebuffer_release(fb);
}
return fb->iPixelFormat;
return iPixelFormat;
}
@ -406,10 +447,10 @@ stw_swap_buffers(
if (fb == NULL)
return FALSE;
if (!(fb->pfi->pfd.dwFlags & PFD_DOUBLEBUFFER))
if (!(fb->pfi->pfd.dwFlags & PFD_DOUBLEBUFFER)) {
stw_framebuffer_release(fb);
return TRUE;
pipe_mutex_lock( fb->mutex );
}
/* If we're swapping the buffer associated with the current context
* we have to flush any pending rendering commands first.
@ -420,7 +461,7 @@ stw_swap_buffers(
if(!st_get_framebuffer_surface( fb->stfb, ST_SURFACE_BACK_LEFT, &surface )) {
/* FIXME: this shouldn't happen, but does on glean */
pipe_mutex_unlock( fb->mutex );
stw_framebuffer_release(fb);
return FALSE;
}
@ -434,8 +475,7 @@ stw_swap_buffers(
stw_dev->stw_winsys->flush_frontbuffer( screen, surface, hdc );
stw_framebuffer_update(fb);
pipe_mutex_unlock( fb->mutex );
stw_framebuffer_release(fb);
return TRUE;
}

View File

@ -41,6 +41,23 @@ struct stw_pixelformat_info;
*/
struct stw_framebuffer
{
/**
* This mutex has two purposes:
* - protect the access to the mutable data members below
* - prevent the the framebuffer from being deleted while being accessed.
*
* It is OK to lock this mutex while holding the stw_device::fb_mutex lock,
* but the opposite must never happen.
*/
pipe_mutex mutex;
/*
* Immutable members.
*
* Note that even access to immutable members implies acquiring the mutex
* above, to prevent the framebuffer from being destroyed.
*/
HDC hDC;
HWND hWnd;
@ -48,7 +65,10 @@ struct stw_framebuffer
const struct stw_pixelformat_info *pfi;
GLvisual visual;
pipe_mutex mutex;
/*
* Mutable members.
*/
struct st_framebuffer *stfb;
/* FIXME: Make this work for multiple contexts bound to the same framebuffer */
@ -56,15 +76,52 @@ struct stw_framebuffer
unsigned width;
unsigned height;
/** This is protected by stw_device::mutex, not the mutex above */
/**
* This is protected by stw_device::fb_mutex, not the mutex above.
*
* Deletions must be done by first acquiring stw_device::fb_mutex, and then
* acquiring the stw_framebuffer::mutex of the framebuffer to be deleted.
* This ensures that nobody else is reading/writing to the.
*
* It is not necessary to aquire the mutex above to navigate the linked list
* given that deletions are done with stw_device::fb_mutex held, so no other
* thread can delete.
*/
struct stw_framebuffer *next;
};
/**
* Create a new framebuffer object which will correspond to the given HDC.
*
* This function will acquire stw_framebuffer::mutex. stw_framebuffer_release
* must be called when done
*/
struct stw_framebuffer *
stw_framebuffer_create_locked(
stw_framebuffer_create(
HDC hdc,
int iPixelFormat );
/**
* Search a framebuffer with a matching HWND.
*
* This function will acquire stw_framebuffer::mutex. stw_framebuffer_release
* must be called when done
*/
struct stw_framebuffer *
stw_framebuffer_from_hwnd(
HWND hwnd );
/**
* Search a framebuffer with a matching HDC.
*
* This function will acquire stw_framebuffer::mutex. stw_framebuffer_release
* must be called when done
*/
struct stw_framebuffer *
stw_framebuffer_from_hdc(
HDC hdc );
BOOL
stw_framebuffer_allocate(
struct stw_framebuffer *fb );
@ -73,15 +130,19 @@ void
stw_framebuffer_update(
struct stw_framebuffer *fb);
/**
* Release stw_framebuffer::mutex lock. This framebuffer must not be accessed
* after calling this function, as it may have been deleted by another thread
* in the meanwhile.
*/
void
stw_framebuffer_release(
struct stw_framebuffer *fb);
/**
* Cleanup any existing framebuffers when exiting application.
*/
void
stw_framebuffer_cleanup(void);
struct stw_framebuffer *
stw_framebuffer_from_hdc_locked(
HDC hdc );
struct stw_framebuffer *
stw_framebuffer_from_hdc(
HDC hdc );
#endif /* STW_FRAMEBUFFER_H */