You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2013/11/20 18:00:05 UTC
Re: svn commit: r1540590 - bad use of SVN_INT_ERR
> URL: http://svn.apache.org/r1540590
> Added: subversion/trunk/subversion/libsvn_subr/object_pool.c
> ==============================================================================
[...]
> +/* Cleanup function called when an object_ref_t gets released.
> + */
> +static apr_status_t
> +object_ref_cleanup(void *baton)
> +{
> + object_ref_t *object = baton;
> + svn_object_pool__t *object_pool = object->object_pool;
> +
> + SVN_INT_ERR(svn_mutex__lock(object_pool->mutex));
This doesn't do quite what you might think. (And it's doc string is not clear about this.) It also prints out the error message. I don't think that's what you want here.
[...]
> + SVN_INT_ERR(svn_mutex__unlock(object_pool->mutex, NULL));
And again.
- Julian
Re: svn commit: r1540590 - bad use of SVN_INT_ERR
Posted by Julian Foad <ju...@btopenworld.com>.
> On Wed, Nov 20, 2013 at 6:00 PM, Julian Foad wrote:
>>> + SVN_INT_ERR(svn_mutex__lock(object_pool->mutex));
>>
>> This doesn't do quite what you might think. (And it's doc string is not
>> clear about this.) It also prints out the error message. [...]
I'll update the doc string for clarity. I'm also going to mark this macro as deprecated, since it no longer suits the needs of any of own command-line programs and it is not really useful to any third-party either, mainly because it always writes 'svn:' as the prefix.
Committed r1544142.
- Julian
Re: svn commit: r1540590 - bad use of SVN_INT_ERR
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Nov 20, 2013 at 6:00 PM, Julian Foad <ju...@btopenworld.com>wrote:
> > URL: http://svn.apache.org/r1540590
>
>
> > Added: subversion/trunk/subversion/libsvn_subr/object_pool.c
> >
> ==============================================================================
> [...]
> > +/* Cleanup function called when an object_ref_t gets released.
> > + */
> > +static apr_status_t
> > +object_ref_cleanup(void *baton)
> > +{
> > + object_ref_t *object = baton;
> > + svn_object_pool__t *object_pool = object->object_pool;
> > +
> > + SVN_INT_ERR(svn_mutex__lock(object_pool->mutex));
>
> This doesn't do quite what you might think. (And it's doc string is not
> clear about this.) It also prints out the error message. I don't think
> that's what you want here.
>
Thanks for pointing that out, Julian!
> [...]
> > + SVN_INT_ERR(svn_mutex__unlock(object_pool->mutex, NULL));
>
> And again.
>
I think there is no real point in continuing program execution once
mutexes / thread synchronization fail in resource management code.
r1544027 attempts a clean exit in that case.
-- Stefan^2.