Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Russ Allbery]

Zack Weinberg <zack@codesourcery.com> writes:

> Consider: There is no guarantee that close will detect errors.  Only
> NFS and Coda implement f_op->flush methods.

And AFS, I believe.  (Not in the standard kernel, of course.)

-- 
Russ Allbery (rra@stanford.edu)             <http://www.eyrie.org/~eagle/>

Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks

[Thunder from the hill]

Hi,

On Tue, 16 Jul 2002, Matthias Andree wrote:
> Indeed, but OTOH, what error is close to report when the file is opened
> read-only?

Well, you can still get EIO, EINTR, EBADF. Whatever you say, disregarding 
the close return code is never any good.

							Regards,
							Thunder
-- 
(Use http://www.ebb.org/ungeek if you can't decode)
------BEGIN GEEK CODE BLOCK------
Version: 3.12
GCS/E/G/S/AT d- s++:-- a? C++$ ULAVHI++++$ P++$ L++++(+++++)$ E W-$
N--- o?  K? w-- O- M V$ PS+ PE- Y- PGP+ t+ 5+ X+ R- !tv b++ DI? !D G
e++++ h* r--- y- 
------END GEEK CODE BLOCK------

close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Zack Weinberg]

Thunder wrote:
> On Tue, 16 Jul 2002, Matthias Andree wrote:
> > Indeed, but OTOH, what error is close to report when the file is
> > opened read-only?
>
> Well, you can still get EIO, EINTR, EBADF. Whatever you say,
> disregarding the close return code is never any good.

Making use of the close return value is also never any good.

Consider: There is no guarantee that close will detect errors.  Only
NFS and Coda implement f_op->flush methods.  For files on all other
file systems, sys_close will always return success (assuming the file
descriptor was open in the first place); the data may still be sitting
in the page cache.  If you need the data pushed to the physical disk,
you have to call fsync.

Consider: If you have called fsync, and it returned successfully, an
immediate call to close is guaranteed to return successfully.  (Any
hypothetical f_op->flush method would have nothing to do; if not, that
filesystem does not correctly implement fsync.)

Therefore, I would argue that it is wrong for any application ever to
inspect close's return value.  Either the program does not need data
integrity guarantees, or it should be using fsync and paying attention
to that instead.

There's also an ugly semantic bind if you make close detect errors.
If close returns an error other than EBADF, has that file descriptor
been closed?  The standards do not specify.  If it has not been
closed, you have a descriptor leak.  But if it has been closed, it is
too late to recover from the error.  [As far as I know, Unix
implementations generally do close the descriptor.]

The manpage that was quoted earlier in this thread is incorrect in
claiming that errors will be detected by close; it should be fixed.

zw

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Alan Cox]

On Wed, 2002-07-17 at 00:22, Zack Weinberg wrote:
> Making use of the close return value is also never any good.

This is untrue

> Consider: There is no guarantee that close will detect errors.  Only
> NFS and Coda implement f_op->flush methods.  For files on all other
> file systems, sys_close will always return success (assuming the file
> descriptor was open in the first place); the data may still be sitting
> in the page cache.  If you need the data pushed to the physical disk,
> you have to call fsync.

close() checking is not about physical disk guarantees. It's about more
basic "I/O completed". In some future Linux only close() might tell you
about some kinds of I/O error. The fact it doesn't do it now is no
excuse for sloppy programming

> There's also an ugly semantic bind if you make close detect errors.
> If close returns an error other than EBADF, has that file descriptor
> been closed?  The standards do not specify.  If it has not been
> closed, you have a descriptor leak.  But if it has been closed, it is
> too late to recover from the error.  [As far as I know, Unix
> implementations generally do close the descriptor.]

If it bothers you close it again 8)

> The manpage that was quoted earlier in this thread is incorrect in
> claiming that errors will be detected by close; it should be fixed.

The man page matches the stsndard. Implementation may be a subset of the
allowed standard right now, but don't program to implementation
assumptions, it leads to nasty accidents

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Zack Weinberg]

On Wed, Jul 17, 2002 at 02:03:02AM +0100, Alan Cox wrote:
> On Wed, 2002-07-17 at 00:22, Zack Weinberg wrote:
> > Making use of the close return value is also never any good.
> 
> This is untrue

I beg to differ.

> > Consider: There is no guarantee that close will detect errors.  Only
> > NFS and Coda implement f_op->flush methods.  For files on all other
> > file systems, sys_close will always return success (assuming the file
> > descriptor was open in the first place); the data may still be sitting
> > in the page cache.  If you need the data pushed to the physical disk,
> > you have to call fsync.
> 
> close() checking is not about physical disk guarantees. It's about more
> basic "I/O completed". In some future Linux only close() might tell you
> about some kinds of I/O error.

I think we're talking past each other.

My first point is that a portable application cannot rely on close to
detect any error.  Only fsync guarantees to detect any errors at all
(except ENOSPC/EDQUOT, which should come back on write; yes, I know
about the buggy NFS implementations that report them only on close).

My second point, which you deleted, is that if some hypothetical close
implementation reports an error under some circumstances, an
immediately preceding fsync call MUST also report the same error under
the same circumstances.

Therefore, if you've checked the return value of fsync, there's no
point in checking the subsequent close; and if you don't care to call
fsync, the close return value is useless since it isn't guaranteed to
detect anything.

> > There's also an ugly semantic bind if you make close detect errors.
> > If close returns an error other than EBADF, has that file descriptor
> > been closed?  The standards do not specify.  If it has not been
> > closed, you have a descriptor leak.  But if it has been closed, it is
> > too late to recover from the error.  [As far as I know, Unix
> > implementations generally do close the descriptor.]
> 
> If it bothers you close it again 8)

And watch it come back with an error again, repeat ad infinitum?

> > The manpage that was quoted earlier in this thread is incorrect in
> > claiming that errors will be detected by close; it should be fixed.
> 
> The man page matches the stsndard. Implementation may be a subset of the
> allowed standard right now, but don't program to implementation
> assumptions, it leads to nasty accidents

You missed the point.  The manpage asserts that I/O errors are
guaranteed to be detected by close; there is no such guarantee.

zw

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Alan Cox]

On Wed, 2002-07-17 at 01:10, Zack Weinberg wrote:
> My first point is that a portable application cannot rely on close to
> detect any error.  Only fsync guarantees to detect any errors at all
> (except ENOSPC/EDQUOT, which should come back on write; yes, I know
> about the buggy NFS implementations that report them only on close).

They are not buggy merely inconvenient. The reality of the NFS protocol
makes it the only viable way to do it

> My second point, which you deleted, is that if some hypothetical close
> implementation reports an error under some circumstances, an
> immediately preceding fsync call MUST also report the same error under
> the same circumstances.

I can't think of a case I'd disagree

> Therefore, if you've checked the return value of fsync, there's no
> point in checking the subsequent close; and if you don't care to call
> fsync, the close return value is useless since it isn't guaranteed to
> detect anything.

If you don't check the return code it might not detect anything. If you
do check the return code it might detect something. In fact you
contradict yourself IMHO by giving the NFS example.

> > If it bothers you close it again 8)
> 
> And watch it come back with an error again, repeat ad infinitum?

The use of intelligence doesn't help. Come on I know you aren't a cobol
programmer. Check for -EBADF ...

> You missed the point.  The manpage asserts that I/O errors are
> guaranteed to be detected by close; there is no such guarantee.

Disagree. It says

It is quite possible that errors on a  previous  write(2)  operation 
are first  reported  at  the  final  close

Not checking the return value when closing the file may lead to silent
loss of  data.

       A successful close does not guarantee that  the  data  has
       been  successfully  saved  to  disk,  as the kernel defers
       writes. It is not common for a  filesystem  to  flush  the
       buffers  when the stream is closed. If you need to be sure
       that the data is physically stored use fsync(2).  (It will
       depend on the disk hardware at this point.)

None of which guarantee what you say, and which agree about the use of
fsync being appropriate now and then

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Zack Weinberg]

On Wed, Jul 17, 2002 at 02:45:40AM +0100, Alan Cox wrote:
> On Wed, 2002-07-17 at 01:10, Zack Weinberg wrote:
> > My first point is that a portable application cannot rely on close to
> > detect any error.  Only fsync guarantees to detect any errors at all
> > (except ENOSPC/EDQUOT, which should come back on write; yes, I know
> > about the buggy NFS implementations that report them only on close).
> 
> They are not buggy merely inconvenient. The reality of the NFS protocol
> makes it the only viable way to do it

You are referring to the way NFSv2 lacks any way to request space
allocation on the server without also flushing data to disk?  It was
my understanding that NFSv2 clients that did not accept the
performance hit and do all writes synchronously were considered
broken.  (since, for instance, POSIX write-visibility guarantees are
violated if writes are delayed on the client.)

In v3 or v4, the WRITE/COMMIT separation lets the implementor generate
prompt ENOSPC and EDQUOT errors without performance penalty.

Another thing to keep in mind is that an application is often in a
much better position to recover from an error, particularly a
disk-full error, if it's reported on write rather than on close.
That's just a quality-of-implementation question, though.

> > > If it bothers you close it again 8)
> > 
> > And watch it come back with an error again, repeat ad infinitum?
> 
> The use of intelligence doesn't help. Come on I know you aren't a cobol
> programmer. Check for -EBADF ...

I wasn't talking about EBADF.  How does the application know the
kernel will ever succeed in closing the file?

> Disagree. It says
> 
> It is quite possible that errors on a  previous  write(2)  operation 
> are first  reported  at  the  final  close
> 
> Not checking the return value when closing the file may lead to silent
> loss of  data.
> 
>        A successful close does not guarantee that  the  data  has
>        been  successfully  saved  to  disk,  as the kernel defers
>        writes. It is not common for a  filesystem  to  flush  the
>        buffers  when the stream is closed. If you need to be sure
>        that the data is physically stored use fsync(2).  (It will
>        depend on the disk hardware at this point.)
> 
> None of which guarantee what you say, and which agree about the use of
> fsync being appropriate now and then

That is not the text quoted upthread.  Looks like the manpage did get
fixed, although I think the current wording is still suboptimal.

zw

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Rogier Wolff]

Alan Cox wrote:
> > And watch it come back with an error again, repeat ad infinitum?
> 
> The use of intelligence doesn't help. Come on I know you aren't a cobol
> programmer. Check for -EBADF ...

Huh? My mgetty/sendfax setup did something interesting lately.

I had not finished installing it, and I got a fax. It recieved it into
/tmp, tried moving it to /var/spool/fax/incoming, failed, and left the
tempfile in /tmp. It then mailed me about the recieved fax in /tmp. 

This is EXACTLY the intelligent behaviour that an application writer
can chose for when checking for error codes. Especially "don't unlink
your tempfiles" is easy if you get errors on conversion or copying....

			Roger. 


-- 
** R.E.Wolff@BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2137555 **
*-- BitWizard writes Linux device drivers for any device you may have! --*
* There are old pilots, and there are bold pilots. 
* There are also old, bald pilots. 

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Lars Marowsky-Bree]

On 2002-07-16T17:10:32,
   Zack Weinberg <zack@codesourcery.com> said:

> Therefore, if you've checked the return value of fsync, there's no
> point in checking the subsequent close; and if you don't care to call
> fsync, the close return value is useless since it isn't guaranteed to
> detect anything.

There is _always_ a point in checking a return value of non void functions.

EOD.


Sincerely,
    Lars Marowsky-Brée <lmb@suse.de>

-- 
Immortality is an adequate definition of high availability for me.
	--- Gregory F. Pfister

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Thunder from the hill]

Hi,

On Tue, 16 Jul 2002, Zack Weinberg wrote:
> the close return value is useless since it isn't guaranteed to detect
> anything.

"Isn't guaranteed to detect anything" is still a lot more encouraging to 
see if it does detect anything than "Is guaranteed not to detect anything".

							Regards,
							Thunder
-- 
(Use http://www.ebb.org/ungeek if you can't decode)
------BEGIN GEEK CODE BLOCK------
Version: 3.12
GCS/E/G/S/AT d- s++:-- a? C++$ ULAVHI++++$ P++$ L++++(+++++)$ E W-$
N--- o?  K? w-- O- M V$ PS+ PE- Y- PGP+ t+ 5+ X+ R- !tv b++ DI? !D G
e++++ h* r--- y- 
------END GEEK CODE BLOCK------

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Elladan]

On Wed, Jul 17, 2002 at 02:03:02AM +0100, Alan Cox wrote:
> On Wed, 2002-07-17 at 00:22, Zack Weinberg wrote:
> 
> > There's also an ugly semantic bind if you make close detect errors.
> > If close returns an error other than EBADF, has that file descriptor
> > been closed?  The standards do not specify.  If it has not been
> > closed, you have a descriptor leak.  But if it has been closed, it is
> > too late to recover from the error.  [As far as I know, Unix
> > implementations generally do close the descriptor.]
> 
> If it bothers you close it again 8)

Consider:

Two threads share the file descriptor table.  

  1. Thread 1 performs close() on a file descriptor.  close fails.
  2. Thread 2 performs open().
* 3. Thread 1 performs close() again, just to make sure.


open() may return any file descriptor not currently in use.

Is step 3 necessary?  Is it dangerous?  The question is, is close
guaranteed to work, or isn't it?


Case 1: Close is guaranteed to close the file.

Thread 2 may have just re-used the file descriptor.  Thus, Thread 1
closes a different file in step 3.  Thread 2 is now using a bad file
descriptor, and becomes very angry because the kernel just said all was
right with the world, and then claims there was a mistake.  Thread 2
leaves in a huff.


Case 2: Close is guaranteed to leave the file open on error.

Thread 2 can't have just re-used the descriptor, so the world is ok in
that sense.  However, Thread 1 *must* perform step 3, or it leaks a
descriptor, the tables fill, and the world becomes a frozen wasteland.


Case 3: Close may or may not leave it open due to random chance or
filesystem peculiarities.

Thread 1 may be required to close it twice, or it may be required not to
close it twice.  It doesn't know!  Night is falling!  The world is in
flames!  Aaaaaaugh!


I believe this demonstrates the need for a standard, one way, or the
other.  :-)

-J

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Thunder from the hill]

Hi,

On Tue, 16 Jul 2002, Elladan wrote:
> Two threads share the file descriptor table.  
> 
>   1. Thread 1 performs close() on a file descriptor.  close fails.
>   2. Thread 2 performs open().
> * 3. Thread 1 performs close() again, just to make sure.

Thread 2 shouldn't be able to reuse a currently open fd. This application 
design is seriously broken.

							Regards,
							Thunder
-- 
(Use http://www.ebb.org/ungeek if you can't decode)
------BEGIN GEEK CODE BLOCK------
Version: 3.12
GCS/E/G/S/AT d- s++:-- a? C++$ ULAVHI++++$ P++$ L++++(+++++)$ E W-$
N--- o?  K? w-- O- M V$ PS+ PE- Y- PGP+ t+ 5+ X+ R- !tv b++ DI? !D G
e++++ h* r--- y- 
------END GEEK CODE BLOCK------

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Elladan]

On Tue, Jul 16, 2002 at 08:54:54PM -0600, Thunder from the hill wrote:
> Hi,
> 
> On Tue, 16 Jul 2002, Elladan wrote:
> > Two threads share the file descriptor table.  
> > 
> >   1. Thread 1 performs close() on a file descriptor.  close fails.
> >   2. Thread 2 performs open().
> > * 3. Thread 1 performs close() again, just to make sure.
> 
> Thread 2 shouldn't be able to reuse a currently open fd. This application 
> design is seriously broken.

No.

Thread 2 doesn't manage the file descriptor table, the kernel does.
Whether the kernel may re-use the descriptor or not depends on whether
the descriptor is closed or not.  The kernel knows, but unless close()
behaves in a defined way, the application does not at this point.  Thus,
step 3 may either be required, forbidden, or undefined.

-J

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Thunder from the hill]

Hi,

On Tue, 16 Jul 2002, Elladan wrote:
> > Thread 2 shouldn't be able to reuse a currently open fd. This application 
> > design is seriously broken.

Okay, again. It's about doing a second close() in case the first one fails 
with EAGAIN. If we have to do it again, the filehandle is not closed, and 
if the filehandle is not closed, the kernel knows that, and if the kernel 
knows that the filehandle is still open, it won't get reassigned. Problem 
gone.

							Regards,
							Thunder
-- 
(Use http://www.ebb.org/ungeek if you can't decode)
------BEGIN GEEK CODE BLOCK------
Version: 3.12
GCS/E/G/S/AT d- s++:-- a? C++$ ULAVHI++++$ P++$ L++++(+++++)$ E W-$
N--- o?  K? w-- O- M V$ PS+ PE- Y- PGP+ t+ 5+ X+ R- !tv b++ DI? !D G
e++++ h* r--- y- 
------END GEEK CODE BLOCK------

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Elladan]

On Tue, Jul 16, 2002 at 09:10:49PM -0600, Thunder from the hill wrote:
> Hi,
> 
> On Tue, 16 Jul 2002, Elladan wrote:
> > > Thread 2 shouldn't be able to reuse a currently open fd. This application 
> > > design is seriously broken.
> 
> Okay, again. It's about doing a second close() in case the first one fails 
> with EAGAIN. If we have to do it again, the filehandle is not closed, and 
> if the filehandle is not closed, the kernel knows that, and if the kernel 
> knows that the filehandle is still open, it won't get reassigned. Problem 
> gone.

This is case 2, "Close is guaranteed to leave the file open on error."

In this case, all applications are required to reissue close commands
upon certain errors, or leak a file descriptor.  This would be a well
defined behavior, though perhaps error prone.

However, note that this is manifestly different from case 1, "Close is
guaranteed to close the file the first time."  If the system behaves via
case 1, closing the handle again is broken as the example illustrated.

The worst, of course, would be undefined behavior for close.  In this
case, the application effectively can't do the right thing without
extreme measures.

-J

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Stevie O]

At 07:22 PM 7/16/2002 -0700, Elladan wrote:
>  1. Thread 1 performs close() on a file descriptor.  close fails.
>  2. Thread 2 performs open().
>* 3. Thread 1 performs close() again, just to make sure.
>
>
>open() may return any file descriptor not currently in use.

I'm confused here... the only way close() can fail is if the file descriptor is invalid (EBADF); wouldn't it be rather stupid to close() a known-to-be-bad descriptor?


--
Stevie-O

Real programmers use COPY CON PROGRAM.EXE

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Elladan]

On Wed, Jul 17, 2002 at 12:17:40AM -0400, Stevie O wrote:
> At 07:22 PM 7/16/2002 -0700, Elladan wrote:
> >  1. Thread 1 performs close() on a file descriptor.  close fails.
> >  2. Thread 2 performs open().
> >* 3. Thread 1 performs close() again, just to make sure.
> >
> >
> >open() may return any file descriptor not currently in use.
> 
> I'm confused here... the only way close() can fail is if the file
> descriptor is invalid (EBADF); wouldn't it be rather stupid to close()
> a known-to-be-bad descriptor?

Well, obviously, if that's the case.  However, the man page for close(2)
doesn't agree (see below).  close() is allowed to return EBADF, EINTR,
or EIO.

The question is, does the OS standard guarantee that the fd is closed,
even if close() returns EINTR or EIO?  Just going by the normal usage of
EINTR, one might think otherwise.  It doesn't appear to be documented
one way or another.

Alan said you could just issue close again to make sure - the example
shows that this is not the case.  A second close is either required or
forbidden in that example - and the behavior has to be well defined or
you won't know which to do.

-J

NAME
       close - close a file descriptor

SYNOPSIS
       #include <unistd.h>

       int close(int fd);

DESCRIPTION
       close closes a file descriptor, so that it no longer refers
       to any file and may be reused. Any locks held on the file it
       was associated with, and owned by the process, are removed
       (regardless of the file descriptor that was used to obtain the
       lock).

       If fd is the last copy of a particular file descriptor the
       resources associated with it are freed; if the descriptor was the
       last reference to a file which has been removed using unlink(2)
       the file is deleted.

RETURN VALUE
       close returns zero on success, or -1 if an error occurred.

ERRORS
       EBADF  fd isn't a valid open file descriptor.

       EINTR  The close() call was interrupted by a signal.

       EIO    An I/O error occurred.

CONFORMING TO
       SVr4,  SVID,  POSIX,  X/OPEN,  BSD 4.3.  SVr4 documents an
       additional ENOLINK error condition.

NOTES
       Not checking the return value of close is a common but
       nevertheless serious programming error.  File system
       implementations which use techniques as `write-behind' to
       increase performance may lead to write(2) succeeding, although
       the data has not been written yet.  The error status may be
       reported at a later write operation, but it is guaranteed to be
       reported on closing the file.  Not checking the return value when
       closing the file may lead to silent loss of data.  This can
       especially be observed with NFS and disk quotas.

       A successful close does not guarantee that the data has
       been successfully saved to  disk, as the kernel defers
       writes.  It is not common for a filesystem to flush the
       buffers when the stream is closed. If you need to be sure
       that the data is physically stored use fsync(2) or
       sync(2), they will get you closer to that goal (it will
       depend on the disk hardware at this point).

SEE ALSO
       open(2), fcntl(2), shutdown(2), unlink(2), fclose(3)

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Andreas Schwab]

Elladan <elladan@eskimo.com> writes:

|> On Wed, Jul 17, 2002 at 12:17:40AM -0400, Stevie O wrote:
|> > At 07:22 PM 7/16/2002 -0700, Elladan wrote:
|> > >  1. Thread 1 performs close() on a file descriptor.  close fails.
|> > >  2. Thread 2 performs open().
|> > >* 3. Thread 1 performs close() again, just to make sure.
|> > >
|> > >
|> > >open() may return any file descriptor not currently in use.
|> > 
|> > I'm confused here... the only way close() can fail is if the file
|> > descriptor is invalid (EBADF); wouldn't it be rather stupid to close()
|> > a known-to-be-bad descriptor?
|> 
|> Well, obviously, if that's the case.  However, the man page for close(2)
|> doesn't agree (see below).  close() is allowed to return EBADF, EINTR,
|> or EIO.
|> 
|> The question is, does the OS standard guarantee that the fd is closed,
|> even if close() returns EINTR or EIO?  Just going by the normal usage of
|> EINTR, one might think otherwise.  It doesn't appear to be documented
|> one way or another.

POSIX says the state of the file descriptor when close fails (with errno
!= EBADF) is unspecified, which means:

    The value or behavior may vary among implementations that conform to
    IEEE Std 1003.1-2001. An application should not rely on the existence
    or validity of the value or behavior. An application that relies on
    any particular value or behavior cannot be assured to be portable
    across conforming implementations.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Elladan]

On Wed, Jul 17, 2002 at 04:39:28PM +0200, Andreas Schwab wrote:
> Elladan <elladan@eskimo.com> writes:
> 
> |> On Wed, Jul 17, 2002 at 12:17:40AM -0400, Stevie O wrote:
> |> > At 07:22 PM 7/16/2002 -0700, Elladan wrote:
> |> > >  1. Thread 1 performs close() on a file descriptor.  close fails.
> |> > >  2. Thread 2 performs open().
> |> > >* 3. Thread 1 performs close() again, just to make sure.
> |> > >
> |> > >
> |> > >open() may return any file descriptor not currently in use.
> |> > 
> |> > I'm confused here... the only way close() can fail is if the file
> |> > descriptor is invalid (EBADF); wouldn't it be rather stupid to close()
> |> > a known-to-be-bad descriptor?
> |> 
> |> Well, obviously, if that's the case.  However, the man page for close(2)
> |> doesn't agree (see below).  close() is allowed to return EBADF, EINTR,
> |> or EIO.
> |> 
> |> The question is, does the OS standard guarantee that the fd is closed,
> |> even if close() returns EINTR or EIO?  Just going by the normal usage of
> |> EINTR, one might think otherwise.  It doesn't appear to be documented
> |> one way or another.
> 
> POSIX says the state of the file descriptor when close fails (with errno
> != EBADF) is unspecified, which means:
> 
>     The value or behavior may vary among implementations that conform to
>     IEEE Std 1003.1-2001. An application should not rely on the existence
>     or validity of the value or behavior. An application that relies on
>     any particular value or behavior cannot be assured to be portable
>     across conforming implementations.

This doesn't mean an OS shouldn't specify the behavior.  Just because
the cross-platform standard leaves it unspecified doesn't mean the OS
should.

Consider what this says, if a particular OS doesn't pick a standard
which the application can port to.  It means that the *only way* to
correctly close a file descriptor is like this:

int ret;
do {
	ret = close(fd);
} while(ret == -1 && errno != EBADF);

That means, if we get an error, we have to loop until the kernel throws
a BADF error!  We can't detect that the file is closed from any other
error value, because only BADF has a defined behavior.

This would sort of work, though of course be hideous, for a single
threaded app.  Now consider a multithreaded app.  To correctly implement
this we have to lock around all calls to close and
open/socket/dup/pipe/creat/etc...

This is clearly ridiculous, and not at all as intended.  Either standard
will work for an OS (though guaranteeing close the first time is much
simpler all around), but it needs to be specified and stuck to, or you
get horrible things like this to work around a bad spec:


void lock_syscalls();
void unlock_syscalls();

int threadsafe_open(const char *file, int flags, mode_t mode)
{
	int fd;
	lock_syscalls();
	fd = open(file, flags, mode);
	unlock_syscalls();
	return fd;
}

int threadsafe_close(int fd)
{
	int ret;
	lock_syscalls();
	do {
		ret = close(fd);
	} while(ret == -1 && errno != EBADF);
	unlock_syscalls();
	return ret;
}

int threadsafe_socket() ...
int threadsafe_pipe() ...
int threadsafe_dup() ...
int threadsafe_creat() ...
int threadsafe_socketpair() ...
int threadsafe_accept() ...

-J

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Linus Torvalds]

In article <20020717164933.GA2136@eskimo.com>,
Elladan  <elladan@eskimo.com> wrote:
>
>Consider what this says, if a particular OS doesn't pick a standard
>which the application can port to.  It means that the *only way* to
>correctly close a file descriptor is like this:
>
>int ret;
>do {
>	ret = close(fd);
>} while(ret == -1 && errno != EBADF);

NO.

The above is
 (a) not portable
 (b) not current practice

The "not portable" part comes from the fact that (as somebody pointed
out), a threaded environment in which the kernel _does_ close the FD on
errors, the FD may have been validly re-used (by the kernel) for some
other thread, and closing the FD a second time is a BUG.

The "not practice" comes from the fact that applications do not do what
you suggest.

The fact is, what Linux does and has always done is the only reasonable
thing to do: the close _will_ tear down the FD, and the error value is
nothing but a warning to the application that there may still be IO
pending (or there may have been failed IO) on the file that the (now
closed) descriptor pointed to.

The application may want to take evasive action (ie try to write the
file again, make a backup, or just warn the user), but the file
descriptor is _gone_. 

>That means, if we get an error, we have to loop until the kernel throws
>a BADF error!  We can't detect that the file is closed from any other
>error value, because only BADF has a defined behavior.

But your loop is _provably_ incorrect for a threaded application.  Your
explicit system call locking approach doesn't work either, because I'm
pretty certain that POSIX already states that open/close are thread
safe, so you can't just invalidate that _other_ standard. 

		Linus

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Elladan]

On Wed, Jul 17, 2002 at 05:43:57PM +0000, Linus Torvalds wrote:
> In article <20020717164933.GA2136@eskimo.com>,
> Elladan  <elladan@eskimo.com> wrote:
> >
> >Consider what this says, if a particular OS doesn't pick a standard
> >which the application can port to.  It means that the *only way* to
> >correctly close a file descriptor is like this:
> >
> >int ret;
> >do {
> >	ret = close(fd);
> >} while(ret == -1 && errno != EBADF);
> 
> NO.
> 
> The above is
>  (a) not portable
>  (b) not current practice
> 
> The "not portable" part comes from the fact that (as somebody pointed
> out), a threaded environment in which the kernel _does_ close the FD on
> errors, the FD may have been validly re-used (by the kernel) for some
> other thread, and closing the FD a second time is a BUG.

That somebody was me.  It appears we're in extremely violent agreement
on this issue.  We both agree the code I wrote is crap.  :-)

-J

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Ketil Froyn]

On Wed, 17 Jul 2002, Linus Torvalds wrote:

> >int ret;
> >do {
> >	ret = close(fd);
> >} while(ret == -1 && errno != EBADF);
>
> NO.
>
> The above is
>  (a) not portable
>  (b) not current practice
>
> The "not portable" part comes from the fact that (as somebody pointed
> out), a threaded environment in which the kernel _does_ close the FD on
> errors, the FD may have been validly re-used (by the kernel) for some
> other thread, and closing the FD a second time is a BUG.
>
> The "not practice" comes from the fact that applications do not do what
> you suggest.
>
> The fact is, what Linux does and has always done is the only reasonable
> thing to do: the close _will_ tear down the FD, and the error value is
> nothing but a warning to the application that there may still be IO
> pending (or there may have been failed IO) on the file that the (now
> closed) descriptor pointed to.

Is this what happens when EINTR is received as well? If so, is there any
point to EINTR? Ie. close() was interrupted, but finished anyway. Would
any application care?

If there is any pending IO when this happens, is it possible to find out
when this is finished? If not, an MTA getting this would have to
temporarily defer the mail it received and hope it doesn't get an EINTR on
close() next time, I guess.

Ketil

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Andries Brouwer]

On Tue, Jul 16, 2002 at 09:38:53PM -0700, Elladan wrote:

> The question is, does the OS standard guarantee that the fd is closed,
> even if close() returns EINTR or EIO?  Just going by the normal usage of
> EINTR, one might think otherwise.  It doesn't appear to be documented
> one way or another.
> 
> Alan said you could just issue close again to make sure - the example
> shows that this is not the case.  A second close is either required or
> forbidden in that example - and the behavior has to be well defined or
> you won't know which to do.

No, the behaviour is not well-defined at all.
The standard explicitly leaves undefined what happens when close returns
EINTR or EIO.

Re: close return value (was Re: [ANNOUNCE] Ext3 vs Reiserfs benchmarks)

[Richard Gooch]

Andries Brouwer writes:
> On Tue, Jul 16, 2002 at 09:38:53PM -0700, Elladan wrote:
> 
> > The question is, does the OS standard guarantee that the fd is closed,
> > even if close() returns EINTR or EIO?  Just going by the normal usage of
> > EINTR, one might think otherwise.  It doesn't appear to be documented
> > one way or another.
> > 
> > Alan said you could just issue close again to make sure - the example
> > shows that this is not the case.  A second close is either required or
> > forbidden in that example - and the behavior has to be well defined or
> > you won't know which to do.
> 
> No, the behaviour is not well-defined at all.
> The standard explicitly leaves undefined what happens when close
> returns EINTR or EIO.

However, the only sane thing to do is to explicitly define one way or
another. The standard is broken. Consider a threaded application,
where one thread tries to call close(), gets an error and re-tries,
because it's not sure if the fd was closed or not. If the fd *is*
closed, and the thread loops calling close(), checking for EBADF,
there is a race if another thread tries calling open()/creat()/dup().

The ambiguity in the standard thus results in the impossibility of
writing a race-free application. And no, forcing the application to
protect system calls with mutexes isn't a solution.

Linux should define explicitly what happens on error return from
close(). Let that be the new standard.

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca