[Dnsmasq-discuss] Is wrapping close() in retry_send() required ?
Simon Kelley
simon at thekelleys.org.uk
Fri Mar 29 21:40:10 GMT 2019
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.
More information about the Dnsmasq-discuss
mailing list