On 26/03/2019 19:33, Pali Rohár wrote:
> On Wednesday 27 February 2019 17:07:21 Simon Kelley wrote:
>> On 27/02/2019 15:06, Bogdan Harjoc wrote:
>>> There are 50 calls to close() in dnsmasq-2.80, and 10 of them are
>>> wrapped in retry_send().
>>>
>>> "man close" has this paragraph in the section "Dealing with error
>>> returns from close":
>>>
>>> "Retrying the close() after a failure return is the wrong thing to do,
>>> since this may cause a reused file descriptor from another thread to
>>> be closed. This can occur because the Linux kernel always releases the
>>> file descriptor early in the close operation, freeing it for reuse;
>>> the steps that may return an error, such as flushing data to the
>>> filesystem or device, occur only later in the close operation".
>>>
>>> Dnsmasq is single-threaded so the retry_send() call is probably
>>> harmless. Are there other OSes that may return an error before the fd
>>> is released, in other words is there an OS where wrapping close in
>>> retry_send is required ?
>>
>>
>> Good questions.
>>
>> Note that retry_send() only deals with EINTR return codes, ie
>> interrupted system calls. (Ok there are other return codes in there, but
>> nothing which might be returned by close())
>>
>> So the use of retry_send(close(...)) is simply to restart the close()
>> syscall if a signal arrives during the syscall.
>>
>>
>> HOWEVER, whilst the man page for close() on my Linux machine states that
>> EINTR is a possible error return, man (7) signal does NOT include
>> close() in the set of syscalls which can be interrupted.
>>
>> Clearly I was reading the close() man page when I used the wrapper, and
>> signal man page when I didn't :)
>>
>>
>> You're probably right that it doesn't matter, but it would be nice to
>> make this at least consistent.
>>
>> Anyone know the answer?
> 
> See manpage: http://man7.org/linux/man-pages/man2/close.2.html
> 
> 
>     The EINTR error is a somewhat special case.  Regarding the EINTR
>     error, POSIX.1-2013 says:
> 
>            If close() is interrupted by a signal that is to be caught, it
>            shall return -1 with errno set to EINTR and the state of
>            fildes is unspecified.
> 
>     This permits the behavior that occurs on Linux and many other
>     implementations, where, as with other errors that may be reported by
>     close(), the file descriptor is guaranteed to be closed.  However, it
>     also permits another possibility: that the implementation returns an
>     EINTR error and keeps the file descriptor open.  (According to its
>     documentation, HP-UX's close() does this.)  The caller must then once
>     more use close() to close the file descriptor, to avoid file
>     descriptor leaks.  This divergence in implementation behaviors
>     provides a difficult hurdle for portable applications, since on many
>     implementations, close() must not be called again after an EINTR
>     error, and on at least one, close() must be called again.  There are
>     plans to address this conundrum for the next major release of the
>     POSIX.1 standard.
> 
> 
> So retrying close() on EINTR on Linux is an application bug. On the
> other hand it is required on HP-UX.
> 
>

Thanks Pali, that looks pretty definitive. As dnsmasq is supported on
Linux but not on HP-UX, I've made the relevant changes.


Cheers,

Simon.


_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Reply via email to