On Fri, 5 Aug 2016 16:00:49 +0100 Tom Hacohen <[email protected]> said:
> This is still racy, just a tiny bit less... If at this point, a lock is > still legal, nothing prevents it from taking the lock (or referencing > the block) just a second afterwards, no? So it still may be broken. > Can't we free a locked lock? I'd imagine that to be possible (though > again, it doesn't solve the issue at hand). this block is on its way to the chopping block. you can have a 0 reffed block but its still in the "read queue" once its read completely and its 0 ref - then it goes off to "free me" land. i was just handling the case where _eina_thread_queue_msg_fetch_done() is called AND we're actually using atomics, so no locks are taken at all, then the block is freed if ref is 0 and ... it's been finished with on fetch. at the same time _eina_thread_queue_msg_alloc_done still may have a lock on blk->lock_non_0_ref which is is releasing there. there is a theoretical race where we can set ref to 0 THEN release this non 0 ref lock and in theory the reader end might read and free in between the ref-- and the lock release. the added take/releases above ensure that cannot happen as they would block waiting for this last lock release. same for when NOT using atomics we ALSO take/release the lock_ref too and thus it takes/releases that for the same reason when not using atomics. so no the block is not legal when it goes down to 0 refs on the fetch/read end. as the write queue is a series/list of blocks the writer should now be writing to some other block further down the list as this block is 100% read now and has 0 refs (no one still composing a message in the block). > On 05/08/16 15:51, Carsten Haitzler wrote: > > raster pushed a commit to branch master. > > > > http://git.enlightenment.org/core/efl.git/commit/?id=26db5d5056a948dc41da65b88a87e575ed21c1eb > > > > commit 26db5d5056a948dc41da65b88a87e575ed21c1eb > > Author: Carsten Haitzler (Rasterman) <[email protected]> > > Date: Fri Aug 5 23:51:03 2016 +0900 > > > > eina thread queue - ensure blocks cannot be freed until lock+unlock > > > > just in case... check blocks cannot be freed until locked and unlocked > > just in case someone still is holding a lock. > > --- > > src/lib/eina/eina_thread_queue.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/src/lib/eina/eina_thread_queue.c > > b/src/lib/eina/eina_thread_queue.c index 190d772..d5bee95 100644 > > --- a/src/lib/eina/eina_thread_queue.c > > +++ b/src/lib/eina/eina_thread_queue.c > > @@ -137,8 +137,12 @@ _eina_thread_queue_msg_block_new(int size) > > static void > > _eina_thread_queue_msg_block_real_free(Eina_Thread_Queue_Msg_Block *blk) > > { > > + eina_lock_take(&(blk->lock_non_0_ref)); > > + eina_lock_release(&(blk->lock_non_0_ref)); > > eina_lock_free(&(blk->lock_non_0_ref)); > > #ifndef ATOMIC > > + eina_lock_take(&(blk->lock_ref)); > > + eina_lock_release(&(blk->lock_ref)); > > eina_spinlock_free(&(blk->lock_ref)); > > #endif > > free(blk); > > > > > ------------------------------------------------------------------------------ > _______________________________________________ > enlightenment-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) [email protected] ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
