Re: Thread-safe error location in maintainer mode

On 4 March 2015 at 16:48, Philip Martin <philip.martin_at_wandisco.com> wrote:
> Ivan Zhakov <ivan_at_visualsvn.com> writes:
>
>> I was thinking about similar change, but as far I remember it's not
>> safe to use svn_atomic__init_once() in error.c, because
>> svn_atomic__init_once() uses svn_error_*() API. This may lead circular
>> initialization of svn_error_*() tracing infrastructure.
>
> That does need to be considered.
>
> The atomic init callback in this case is written to never return an
> error so the only errors to consider are those raised by
> svn_atomic__init_once itself. Any such error would be a failure of
> svn_atomic_cas. Possible causes: the svn_atomic_t passed in is something
> other than 0/1/2 or the svn_atomic_t was read-only. Anything else?
>
> If I initialize:
>
> static volatile svn_atomic_t init_status = 10;
>
> the client does go into an infinite loop when attempting to raise an
> error but that doesn't have anything to do with this patch, the same
> happens with any call to svn_atomic__init_once.
>
> If I initialize
>
> static volatile svn_atomic_t init_status = 2;
>
This is exactly what I tried two minutes ago.

> the client recurses until it SEGVs when svn_atomic__init_once tries to
> raise an error, that is caused by this patch. The error that
> svn_atomic__init_once is trying to raise would be discarded by
> svn_error__locate.
>
> A memory overwrite bug could cause the svn_atomic_t used by
> svn_error__locate to become 2 but equally a memory overwrite bug could
> set any other svn_atomic_t to 10 and cause an infinite loop. We could
> add a "no errors" flag to svn_atomic__init_once to avoid the
> recursion/SEGV caused when the svn_atomic_t is 2 but trying to handle
> memory overwrite is never going to be reliable.
>
> Overall I don't think the circular problem is a problem in practice.
>
I agree that currently it's not possible to get stackoverflow, but I
prefer to avoid circular dependency anyway.
svn_atomic__init_once_no_error() seems to nice solution for this
_potential_ problem.

-- 
Ivan Zhakov