On 03/30/2011 03:22 PM, Daniel Shahaf wrote:
> Blair Zajac wrote on Wed, Mar 30, 2011 at 15:13:07 -0700:
>> On 03/30/2011 03:06 PM, Daniel Shahaf wrote:
>>> I notice that FILE_LOCK_RETRY_LOOP() checks for EINTR when EDEADLK isn't
>>> defined (or when APR isn't threaded). I assume that check is there for
>>> the same reason --- i.e., that EINTR is another symptom of the deadlock
>>> situation that FILE_LOCK_RETRY_LOOP() was added due to?
>>
>> No, that's unrelated. While I was working on the function, I
>> decided to handle EINTR in the same way that many of our other io
>> functions do, e.g. svn_io_create_unique_link(), svn_io_read_link(),
>> do_io_file_wrapper_cleanup(). This one didn't seem to hurt and
>> makes the function more robust. I didn't want spurious EINTR errors
>> when trying to get a lock.
>>
>
> Okay, thanks. I stand by my +1 then.
>
> (APR already handles EINTR in apr_file_lock(), at least in some
> environments; but I don't see the harm in leaving our check in too.)
Good catch, I didn't check apr_file_lock()'s implementation. We could
remove that if all environments check for it, but it's definitely safer
having svn do it in case there was a time when APR didn't, but APR 0.9.x
does retry also.
Blair
Received on 2011-03-31 02:41:41 CEST