# HG changeset patch # User Tero Marttila # Date 1213715108 -10800 # Node ID 7512207c9041cd7b09b0d193838ef3df8e0a9fea # Parent d18606bb6f2052b5370e1e55b80227d58fea70ec write a DEBUG macro and change the render_threads stuff to use it. Shuffle the mutexes in render_threads around again to fix yet another bug... seems to work, but meh committer: Tero Marttila diff -r d18606bb6f20 -r 7512207c9041 common.h --- a/common.h Tue Jun 17 16:39:55 2008 +0300 +++ b/common.h Tue Jun 17 18:05:08 2008 +0300 @@ -3,14 +3,25 @@ * error handling */ -void _generic_err (const char *func, int perr, const char *fmt, ...) +void _generic_err ( /*int level, */ const char *func, int perr, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))); // needs to be defined as its own function for the noreturn attribute -void _generic_err_exit (const char *func, int perr, const char *fmt, ...) +void _generic_err_exit ( /* int level, */ const char *func, int perr, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))) __attribute__ ((noreturn)); +/* +enum _debug_level { + DEBUG_FATAL, + DEBUG_ERROR, + DEBUG_WARNING, + DEBUG_INFO, + DEBUG_DEBUG, +}; + +extern enum _debug_level _cur_debug_level; +*/ // various kinds of ways to handle an error, 2**3 of them, *g* #define error(...) _generic_err( NULL, 0, __VA_ARGS__) @@ -30,6 +41,12 @@ #define WARNING(...) err_func(__func__, __VA_ARGS__) #define PWARNING(...) perr_func(__func__, __VA_ARGS__) +#ifdef DEBUG_ENABLED +#define DEBUG(...) err_func(__func__, __VA_ARGS__) +#else +#define DEBUG(...) if (0) { } +#endif + /* * Parse a host:port string. * diff -r d18606bb6f20 -r 7512207c9041 render_threads.c --- a/render_threads.c Tue Jun 17 16:39:55 2008 +0300 +++ b/render_threads.c Tue Jun 17 18:05:08 2008 +0300 @@ -3,6 +3,8 @@ #include +#define DEBUG_ENABLED + #include "common.h" #include "render_threads.h" #include "render_slices_struct.h" @@ -135,8 +137,17 @@ // used to protect slices_info pthread_mutex_t slices_mut; + + /* + * Return value from the manager thread + */ + enum render_threads_retval { + RETVAL_SUCCESS, + RETVAL_FAILURE, + } manager_retval; }; + void render_threads_deinit (struct render_threads *ctx) { render_slices_deinit(&ctx->slices_info); } @@ -151,6 +162,9 @@ int _render_threads_slice_row (void *arg, unsigned char *rowbuf) { struct render_thread *subctx = arg; int status, i; + + // test for cancelation, we don't hold any locks right now + pthread_testcancel(); // we need to handle the render_slices state atomically, so lock it pthread_mutex_lock(&subctx->self->slices_mut); @@ -163,23 +177,15 @@ status = render_slices_segment_done(&subctx->self->slices_info, subctx->slice_info->index); // debugging -/* subctx->segments_done++; - printf("slice %zu: segment_done, status=(%s %s), row=%d\n", subctx->slice_info->index, + DEBUG("slice %zu: segment_done, status=(%s %s), row=%d", subctx->slice_info->index, status & SLICE_PROCESS_ROW ? "SLICE_PROCESS_ROW" : "", status & SLICE_CONTINUE ? "SLICE_CONTINUE" : "", - subctx->segments_done + ++subctx->segments_done ); -*/ - - // done with the slices stuff - pthread_mutex_unlock(&subctx->self->continue_mut); - pthread_mutex_unlock(&subctx->self->slices_mut); // do we need to notify the other worker threads? if (status & SLICE_CONTINUE) { - pthread_mutex_lock(&subctx->self->continue_mut); - -// printf("slice %zu: signal continue\n", subctx->slice_info->index); + DEBUG("slice %zu: signal continue", subctx->slice_info->index); // mark them as continueable for (i = 0; i < subctx->self->slice_count; i++) @@ -187,10 +193,12 @@ // signal then to continue pthread_cond_broadcast(&subctx->self->continue_cond); + } - pthread_mutex_unlock(&subctx->self->continue_mut); - } - + // done with the slices stuff + pthread_mutex_unlock(&subctx->self->continue_mut); + pthread_mutex_unlock(&subctx->self->slices_mut); + // tell the manager thread that it can process a row if (status & SLICE_PROCESS_ROW) { // grab the process_row lock so that we can do stuff with it @@ -205,38 +213,43 @@ // release the process_row mutex pthread_mutex_unlock(&subctx->self->process_row_mut); } - - // handle our can-continue status - pthread_mutex_lock(&subctx->self->continue_mut); - - if (status & SLICE_CONTINUE) { - // don't wait for anything, we can just continue right away -// printf("slice %zu: direct continue\n", subctx->slice_info->index); - } else { - if (!subctx->can_continue) { - // we need to wait until someone signals it -// printf("slice %zu: waiting...\n", subctx->slice_info->index); +#define BEGIN_MUTEX(mut) pthread_cleanup_push((void (*)(void *)) pthread_mutex_unlock, &(mut)); pthread_mutex_lock(&(mut)); +#define END_MUTEX pthread_cleanup_pop(1); - pthread_cond_wait(&subctx->self->continue_cond, &subctx->self->continue_mut); - -// printf("slice %zu: continue after wait\n", subctx->slice_info->index); + // handle our can-continue status + // we need to release this lock if we get canceled in pthread_cond_wait + BEGIN_MUTEX(subctx->self->continue_mut) + if (status & SLICE_CONTINUE) { + // don't wait for anything, we can just continue right away + DEBUG("slice %zu: direct continue", subctx->slice_info->index); } else { - // no need to wait, because someone else took care of that before we got a chance to wait -// printf("slice %zu: indirect continue\n", subctx->slice_info->index); - + if (!subctx->can_continue) { + // we need to wait until someone signals it + DEBUG("slice %zu: waiting...", subctx->slice_info->index); + + pthread_cond_wait(&subctx->self->continue_cond, &subctx->self->continue_mut); + + DEBUG("slice %zu: continue after wait", subctx->slice_info->index); + + } else { + // no need to wait, because someone else took care of that before we got a chance to wait + DEBUG("slice %zu: indirect continue", subctx->slice_info->index); + + } } - } - - // we should now be marked as continueable - assert(subctx->can_continue); + + // we should now be marked as continueable + assert(subctx->can_continue); - // proceed to continue, so clear this - subctx->can_continue = 0; - - // done handling the continue state - pthread_mutex_unlock(&subctx->self->continue_mut); + // proceed to continue, so clear this + subctx->can_continue = 0; + + // done handling the continue state + pthread_mutex_unlock(&subctx->self->continue_mut); + + END_MUTEX // row handled succesfully return 0; @@ -260,13 +273,13 @@ pthread_mutex_unlock(&subctx->self->continue_mut); // start rendering... -// printf("slice %zu: start\n", subctx->slice_info->index); + DEBUG("slice %zu: start", subctx->slice_info->index); if (render_mandelbrot(subctx->slice_info->render_info)) goto error; // success, slice render complete -// printf("slice %zu: done\n", subctx->slice_info->index); + DEBUG("slice %zu: done", subctx->slice_info->index); // sanity checks assert(subctx->self->slices_running > 0); @@ -275,7 +288,7 @@ if (--subctx->self->slices_running == 0) { // this was the last row, send the manager a final signal in case it's waiting -// printf("slice %zu: signal\n", subctx->slice_info->index); + DEBUG("slice %zu: signal", subctx->slice_info->index); pthread_cond_signal(&subctx->self->process_row_cond); } @@ -307,12 +320,12 @@ // then we wait around and call process_row when told to do { // the process_row must be locked here -// printf("manager: to wait\n"); + DEBUG("manager: to wait"); // figure out if we need to wait if (ctx->waiting_rows == 0 && ctx->slices_running > 0) { // no work to do right now, so wait for some -// printf("manager: waiting\n"); + DEBUG("manager: waiting"); // wait until a worker thread signals us pthread_cond_wait(&ctx->process_row_cond, &ctx->process_row_mut); @@ -338,18 +351,14 @@ // call into render_slices status = render_slices_process_row(&ctx->slices_info); -/* printf("manager: did process_row, status=(%s %s)\n", + DEBUG("manager: did process_row, status=(%s %s)", status & SLICE_PROCESS_ROW ? "SLICE_PROCESS_ROW" : "", status & SLICE_CONTINUE ? "SLICE_CONTINUE" : "" ); -*/ - - // done with render_slices - pthread_mutex_unlock(&ctx->slices_mut); - - + // any errors? if (status == -1) { + pthread_mutex_unlock(&ctx->slices_mut); goto error; } @@ -358,11 +367,11 @@ // grab the continue_mut lock while we update this state pthread_mutex_lock(&ctx->continue_mut); - // increment continue_count for all slices + // set can_continue for all slices for (i = 0; i < ctx->slice_count; i++) ctx->threads[i].can_continue = 1; -// printf("manager: signal continue\n"); + DEBUG("manager: signal continue"); // signal any waiting worker threads to continue, giving them a chance to call cond_wait first. pthread_cond_broadcast(&ctx->continue_cond); @@ -370,6 +379,9 @@ // done with that pthread_mutex_unlock(&ctx->continue_mut); } + + // done with render_slices + pthread_mutex_unlock(&ctx->slices_mut); // XXX: ignore SLICE_PROCESS_ROW @@ -392,25 +404,47 @@ if (render_slices_done(&ctx->slices_info)) goto error; -// printf("manager: done\n"); + DEBUG("manager: done"); - // reap all the workers void *retval; + // reap all the workers for (i = 0; i < ctx->slice_count; i++) { if (pthread_join(ctx->threads[i].thread, &retval)) PERROR("pthread_join"); - // XXX: assume they don't get canceled (SIGSEV etc?) + // XXX: assume they don't get canceled assert(retval == NULL); } - - // ok, exit ourself - pthread_exit(NULL); + + // succesfull return + ctx->manager_retval = RETVAL_SUCCESS; + + goto exit; error: - /* XXX: handle errors gracefully... */ - FATAL("the threaded render operation failed, taking down the rest of the daemon application as well..."); + /* cancel and join all worker threads */ + WARNING("canceling and reaping all threads"); + + for (i = 0; i < ctx->slice_count; i++) { + if (pthread_cancel(ctx->threads[i].thread)) + PWARNING("pthread_cancel(worker:%d)", i); + else { + if (pthread_join(ctx->threads[i].thread, &retval)) { + PWARNING("pthread_join(worker:%d)", i); + } else { + if (retval != PTHREAD_CANCELED) + PWARNING("worker:%d retval = %p", i, retval); + } + } + } + + // failure + ctx->manager_retval = RETVAL_FAILURE; + +exit: + // ok, exit ourself + pthread_exit(ctx); } int render_threads_init (struct render_threads *ctx, struct render *render_info) { @@ -494,8 +528,13 @@ if (retval == PTHREAD_CANCELED) ERROR("manager thread was canceled"); - else if (retval != NULL) + else if (retval != ctx) ERROR("manager thread exited with unknown return value"); + else { + if (ctx->manager_retval != RETVAL_SUCCESS) { + ERROR("manger thread exited with an error"); + } + } // ok, success return 0;