Re: [Dnsmasq-discuss] Is wrapping close() in retry_send() required ?
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
Re: [Dnsmasq-discuss] Is wrapping close() in retry_send() required ?
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. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: PGP signature ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] Is wrapping close() in retry_send() required ?
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? Cheers, Simon. ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
