bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: double _close()?


From: Bruno Haible
Subject: Re: double _close()?
Date: Thu, 14 Oct 2021 03:18:11 +0200

Hi Gisle,

> I've been trying to speed-up a 'diff -r dir1 dir2'
> by disabling the "no-throw" stuff for MSVC/clang-cl:
>    MSVC_INVALID_PARAMETER_HANDLING == SANE_LIBRARY_HANDLING
> 
> (suspecting it imposes some overhead. Maybe MSVC's
> __try/__catch has lower overhead?)
> ...
> Or are we discouraged to
> disable this TRY/CATCH stuff?

I added this __try/__catch uses because
  - POSIX wants an error return rather than a program crash for
    calls like close(-1).
  - We make extensive use of such calls in the test suite.

If you find, by extensive testing, that SANE_LIBRARY_HANDLING works
for your programs, you can of course enable it.

However, that will not bring a significant performance benefit,
since installing a handler is cheap. It's the actual response to
an exception (with stack unwinding) that's expensive; but at this
point you will already have verified that no such exception is
being generated.

The actual performance improvement comes with the patches you
propose to eliminate the exceptions. This one for example:

> #if WINDOWS_SOCKETS
>    /* Call the overridden close(), then the original fclose().
>       Note about multithread-safety: There is a race condition where some
>       other thread could open fd between our close and fclose.  */
>    if (close (fd) < 0 && saved_errno == 0)
>      saved_errno = errno;
> 
>    fclose_nothrow (fp); /* will fail with errno = EBADF,
>                            if we did not lose a race */
> ---------
> 
> the 'fp' associated with the 'fd' got closed.
> 
> I did this to fix it:
> 
> --- a/fclose.c 2021-06-10 12:59:56
> +++ b/fclose.c 2021-10-12 13:43:18
> @@ -79,9 +79,11 @@
>     /* Call the overridden close(), then the original fclose().
>        Note about multithread-safety: There is a race condition where some
>        other thread could open fd between our close and fclose.  */
> -  if (close (fd) < 0 && saved_errno == 0)
> +  result = close (fd);
> +  if (result < 0 && saved_errno == 0)
>       saved_errno = errno;
> 
> +  /* If the above 'close()' succeeded, don't close the associated 'fp' 
> again. */
> +  if (result)
>       fclose_nothrow (fp); /* will fail with errno = EBADF,
>                               if we did not lose a race */
> ----------------
> 
> Does this make sense?

It would make sense for the fclose at the end of a program.

However, in general, a 'FILE' object often holds a buffer, and
fclose(fp) is supposed to free that buffer, even if fd is already
closed. Thus, skipping the fclose_nothrow call introduces a memory leak.

Bruno






reply via email to

[Prev in Thread] Current Thread [Next in Thread]