Fixes for thread related render / compositing crashes:

* Viewer node could free image while it is being redrawn, viewer image
  buffers now need acquire/release to be accessed as was already the
  case for render results.
* The Composite node could free the image buffers outside of a lock,
  also causing simultaneous redraw to crash.
* Especially on Windows, re-rendering could crash when drawing an image
  that was freed. When RE_RenderInProgress was true it would access the
  image buffer and simply return it while it could still contain a pointer
  to a render result buffer that was already freed. I don't understand
  why this case was there in the first place, so I've removed it.

Possibly fixes bugs #20174, #21418, #21391, #21394.
This commit is contained in:
Brecht Van Lommel 2010-03-16 16:58:45 +00:00
parent 735b444d74
commit f17dcf58c8
6 changed files with 108 additions and 116 deletions

@ -1932,113 +1932,93 @@ static ImBuf *image_get_ibuf_multilayer(Image *ima, ImageUser *iuser)
/* always returns a single ibuf, also during render progress */
static ImBuf *image_get_render_result(Image *ima, ImageUser *iuser, void **lock_r)
{
Render *re= NULL;
RenderResult *rr= NULL;
Render *re;
RenderResult rres;
float *rectf, *rectz;
unsigned int *rect;
float dither;
int channels, layer, pass;
ImBuf *ibuf;
if(!(iuser && iuser->scene))
return NULL;
/* if we the caller is not going to release the lock, don't give the image */
if(!lock_r)
return NULL;
if(iuser && iuser->scene) {
re= RE_GetRender(iuser->scene->id.name, RE_SLOT_VIEW);
rr= RE_AcquireResultRead(re);
re= RE_GetRender(iuser->scene->id.name, RE_SLOT_VIEW);
/* release is done in BKE_image_release_ibuf using lock_r */
*lock_r= re;
}
if(rr==NULL)
return NULL;
channels= 4;
layer= (iuser)? iuser->layer: 0;
pass= (iuser)? iuser->pass: 0;
if(RE_RenderInProgress(re)) {
ImBuf *ibuf= image_get_ibuf(ima, IMA_NO_INDEX, 0);
/* make ibuf if needed, and initialize it */
/* this only gets called when mutex locked */
if(ibuf==NULL) {
ibuf= IMB_allocImBuf(rr->rectx, rr->recty, 32, IB_rect, 0);
image_assign_ibuf(ima, ibuf, IMA_NO_INDEX, 0);
}
/* this gives active layer, composite or seqence result */
RE_AcquireResultImage(re, &rres);
rect= (unsigned int *)rres.rect32;
rectf= rres.rectf;
rectz= rres.rectz;
dither= iuser->scene->r.dither_intensity;
return ibuf;
/* get compo/seq result by default */
if(rres.rectf && layer==0);
else if(rres.layers.first) {
RenderLayer *rl= BLI_findlink(&rres.layers, layer-(rres.rectf?1:0));
if(rl) {
RenderPass *rpass;
/* there's no combined pass, is in renderlayer itself */
if(pass==0) {
rectf= rl->rectf;
}
else {
rpass= BLI_findlink(&rl->passes, pass-1);
if(rpass) {
channels= rpass->channels;
rectf= rpass->rect;
dither= 0.0f; /* don't dither passes */
}
}
for(rpass= rl->passes.first; rpass; rpass= rpass->next)
if(rpass->passtype == SCE_PASS_Z)
rectz= rpass->rect;
}
}
else {
RenderResult rres;
float *rectf, *rectz;
unsigned int *rect;
float dither;
int channels, layer, pass;
channels= 4;
layer= (iuser)? iuser->layer: 0;
pass= (iuser)? iuser->pass: 0;
/* this gives active layer, composite or seqence result */
RE_AcquireResultImage(RE_GetRender(iuser->scene->id.name, RE_SLOT_VIEW), &rres);
rect= (unsigned int *)rres.rect32;
rectf= rres.rectf;
rectz= rres.rectz;
dither= iuser->scene->r.dither_intensity;
/* get compo/seq result by default */
if(rr->rectf && layer==0);
else if(rr->layers.first) {
RenderLayer *rl= BLI_findlink(&rr->layers, layer-(rr->rectf?1:0));
if(rl) {
RenderPass *rpass;
/* there's no combined pass, is in renderlayer itself */
if(pass==0) {
rectf= rl->rectf;
}
else {
rpass= BLI_findlink(&rl->passes, pass-1);
if(rpass) {
channels= rpass->channels;
rectf= rpass->rect;
dither= 0.0f; /* don't dither passes */
}
}
for(rpass= rl->passes.first; rpass; rpass= rpass->next)
if(rpass->passtype == SCE_PASS_Z)
rectz= rpass->rect;
}
}
if(rectf || rect) {
ImBuf *ibuf= image_get_ibuf(ima, IMA_NO_INDEX, 0);
/* make ibuf if needed, and initialize it */
if(ibuf==NULL) {
ibuf= IMB_allocImBuf(rr->rectx, rr->recty, 32, 0, 0);
image_assign_ibuf(ima, ibuf, IMA_NO_INDEX, 0);
}
ibuf->x= rr->rectx;
ibuf->y= rr->recty;
if(ibuf->rect_float!=rectf || rect) /* ensure correct redraw */
imb_freerectImBuf(ibuf);
if(rect)
ibuf->rect= rect;
ibuf->rect_float= rectf;
ibuf->flags |= IB_rectfloat;
ibuf->channels= channels;
ibuf->zbuf_float= rectz;
ibuf->flags |= IB_zbuffloat;
ibuf->dither= dither;
RE_ReleaseResultImage(re);
ima->ok= IMA_OK_LOADED;
return ibuf;
}
if(!(rectf || rect)) {
RE_ReleaseResultImage(re);
return NULL;
}
ibuf= image_get_ibuf(ima, IMA_NO_INDEX, 0);
/* make ibuf if needed, and initialize it */
if(ibuf==NULL) {
ibuf= IMB_allocImBuf(rres.rectx, rres.recty, 32, 0, 0);
image_assign_ibuf(ima, ibuf, IMA_NO_INDEX, 0);
}
ibuf->x= rres.rectx;
ibuf->y= rres.recty;
return NULL;
if(ibuf->rect_float!=rectf || rect) /* ensure correct redraw */
imb_freerectImBuf(ibuf);
if(rect)
ibuf->rect= rect;
ibuf->rect_float= rectf;
ibuf->flags |= IB_rectfloat;
ibuf->channels= channels;
ibuf->zbuf_float= rectz;
ibuf->flags |= IB_zbuffloat;
ibuf->dither= dither;
ima->ok= IMA_OK_LOADED;
/* release is done in BKE_image_release_ibuf using lock_r */
*lock_r= re;
return ibuf;
}
static ImBuf *image_get_ibuf_threadsafe(Image *ima, ImageUser *iuser, int *frame_r, int *index_r)
@ -2199,10 +2179,17 @@ ImBuf *BKE_image_acquire_ibuf(Image *ima, ImageUser *iuser, void **lock_r)
ibuf= image_get_render_result(ima, iuser, lock_r);
}
else if(ima->type==IMA_TYPE_COMPOSITE) {
/* Composite Viewer, all handled in compositor */
/* fake ibuf, will be filled in compositor */
ibuf= IMB_allocImBuf(256, 256, 32, IB_rect, 0);
image_assign_ibuf(ima, ibuf, 0, frame);
/* requires lock/unlock, otherwise don't return image */
if(lock_r) {
/* unlock in BKE_image_release_ibuf */
BLI_lock_thread(LOCK_VIEWER);
*lock_r= ima;
/* Composite Viewer, all handled in compositor */
/* fake ibuf, will be filled in compositor */
ibuf= IMB_allocImBuf(256, 256, 32, IB_rect, 0);
image_assign_ibuf(ima, ibuf, 0, frame);
}
}
}
}
@ -2220,9 +2207,11 @@ ImBuf *BKE_image_acquire_ibuf(Image *ima, ImageUser *iuser, void **lock_r)
void BKE_image_release_ibuf(Image *ima, void *lock)
{
/* for getting image during threaded render, need to release */
if(lock)
RE_ReleaseResult(lock);
/* for getting image during threaded render / compositing, need to release */
if(lock == ima)
BLI_unlock_thread(LOCK_VIEWER); /* viewer image */
else if(lock)
RE_ReleaseResultImage(lock); /* render result */
}
ImBuf *BKE_image_get_ibuf(Image *ima, ImageUser *iuser)

@ -59,7 +59,8 @@ int BLI_system_thread_count(void); /* gets the number of threads the system can
#define LOCK_IMAGE 0
#define LOCK_PREVIEW 1
#define LOCK_CUSTOM1 2
#define LOCK_VIEWER 2
#define LOCK_CUSTOM1 3
void BLI_lock_thread(int type);
void BLI_unlock_thread(int type);

@ -109,6 +109,7 @@ A sample loop can look like this (pseudo c);
static pthread_mutex_t _malloc_lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t _image_lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t _preview_lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t _viewer_lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t _custom1_lock = PTHREAD_MUTEX_INITIALIZER;
static int thread_levels= 0; /* threads can be invoked inside threads */
@ -327,6 +328,8 @@ void BLI_lock_thread(int type)
pthread_mutex_lock(&_image_lock);
else if (type==LOCK_PREVIEW)
pthread_mutex_lock(&_preview_lock);
else if (type==LOCK_VIEWER)
pthread_mutex_lock(&_viewer_lock);
else if (type==LOCK_CUSTOM1)
pthread_mutex_lock(&_custom1_lock);
}
@ -337,6 +340,8 @@ void BLI_unlock_thread(int type)
pthread_mutex_unlock(&_image_lock);
else if (type==LOCK_PREVIEW)
pthread_mutex_unlock(&_preview_lock);
else if (type==LOCK_VIEWER)
pthread_mutex_unlock(&_viewer_lock);
else if(type==LOCK_CUSTOM1)
pthread_mutex_unlock(&_custom1_lock);
}

@ -80,10 +80,10 @@ static void node_composit_exec_composite(void *data, bNode *node, bNodeStack **i
outbuf->malloc= 0;
free_compbuf(outbuf);
RE_ReleaseResult(re);
/* signal for imageviewer to refresh (it converts to byte rects...) */
BKE_image_signal(BKE_image_verify_viewer(IMA_TYPE_R_RESULT, "Render Result"), NULL, IMA_SIGNAL_FREE);
RE_ReleaseResult(re);
return;
}
else

@ -50,13 +50,15 @@ static void node_composit_exec_viewer(void *data, bNode *node, bNodeStack **in,
ImBuf *ibuf;
CompBuf *cbuf, *tbuf;
int rectx, recty;
void *lock;
BKE_image_user_calc_frame(node->storage, rd->cfra, 0);
/* always returns for viewer image, but we check nevertheless */
ibuf= BKE_image_get_ibuf(ima, node->storage);
ibuf= BKE_image_acquire_ibuf(ima, node->storage, &lock);
if(ibuf==NULL) {
printf("node_composit_exec_viewer error\n");
BKE_image_release_ibuf(ima, lock);
return;
}
@ -106,6 +108,8 @@ static void node_composit_exec_viewer(void *data, bNode *node, bNodeStack **in,
free_compbuf(zbuf);
}
BKE_image_release_ibuf(ima, lock);
generate_preview(data, node, cbuf);
free_compbuf(cbuf);

@ -138,11 +138,6 @@ static void int_nothing(void *unused, int val) {}
static void print_error(void *unused, char *str) {printf("ERROR: %s\n", str);}
static int default_break(void *unused) {return G.afbreek == 1;}
int RE_RenderInProgress(Render *re)
{
return re->result_ok==0;
}
static void stats_background(void *unused, RenderStats *rs)
{
uintptr_t mem_in_use= MEM_get_memory_in_use();
@ -1091,6 +1086,8 @@ void RE_AcquireResultImage(Render *re, RenderResult *rr)
if(rr->rectz==NULL)
rr->rectz= RE_RenderLayerGetPass(rl, SCE_PASS_Z);
}
rr->layers= re->result->layers;
}
}
}
@ -2801,7 +2798,6 @@ void RE_BlenderFrame(Render *re, Scene *scene, SceneRenderLayer *srl, unsigned i
{
/* ugly global still... is to prevent preview events and signal subsurfs etc to make full resol */
RenderGlobal.renderingslot= re->slot;
re->result_ok= 0;
G.rendering= 1;
scene->r.cfra= frame;
@ -2811,7 +2807,6 @@ void RE_BlenderFrame(Render *re, Scene *scene, SceneRenderLayer *srl, unsigned i
}
/* UGLY WARNING */
re->result_ok= 1;
G.rendering= 0;
RenderGlobal.renderingslot= RenderGlobal.viewslot;
}
@ -2917,7 +2912,6 @@ void RE_BlenderAnim(Render *re, Scene *scene, unsigned int lay, int sfra, int ef
/* is also set by caller renderwin.c */
G.rendering= 1;
RenderGlobal.renderingslot= re->slot;
re->result_ok= 0;
if(BKE_imtype_is_movie(scene->r.imtype))
if(!mh->start_movie(scene, &re->r, re->rectx, re->recty, reports))
@ -3014,7 +3008,6 @@ void RE_BlenderAnim(Render *re, Scene *scene, unsigned int lay, int sfra, int ef
/* UGLY WARNING */
G.rendering= 0;
re->result_ok= 1;
RenderGlobal.renderingslot= RenderGlobal.viewslot;
}