[collectd] [PATCH] rrdtool plugin: Added a flush callback.

Sebastian Harl sh at tokkee.org
Wed Feb 27 21:36:23 CET 2008


Hi,

On Wed, Feb 27, 2008 at 11:39:19AM +0100, bd at bc-bd.org wrote:
> > +static int rrd_flush (const int timeout)
> > +{
> > +	if (cache == NULL)
> > +		return (0);
> 
> Shouldn't this check be made with the lock held? I am not sure but else
> we might have a race condition here.
> 
> > +	pthread_mutex_lock (&cache_lock);

In theory, you're right. However, the conditional check for 0 should be
an atomic operation on basically any platform. Also, if it's not, the
only "bad" thing that might happen is that we don't flush the cache even
though it got available in this very moment. Once the cache has been
initialized (i.e. it's no longer equal to NULL) it will live forever, so
we'll never get to the point that we'll check the cache to be not equal
to zero and then flush the cache which might have been destroyed in the
meantime.

Anyway, for the sake of correctness and to prevent future errors, I'll
send a patch in a minute.

Thanks for the hint.

Cheers,
Sebastian

-- 
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety.         -- Benjamin Franklin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20080227/ce58ec50/attachment.pgp 


More information about the collectd mailing list