Do multiple close calls for one fd in the same function matter?

15,120

Solution 1

The first call should return 0; the second call should return -1, and set errno to EBADF.

You should prevent the second call from happening by setting fd to a known bad number, e.g. a -1 immediately after the first call of close, and subsequently checking fd before making the second call (and not making the call if fd is -1):

close(fd);
fd = -1;
...
// More code
...
if (fd != -1) {
    close(fd)
    fd = -1;
}

This code pattern will help when you need to make calls to close from multiple places, but you are not sure if the file is open, or if it has been closed already. Passing -1 to close is harmless (you would get an EBADF, of course).

Solution 2

It should be harmless unless you're threaded or doing something between the two calls to close. Then you might end up closing an fd that something else in your program has opened.

The way threading is relevant is that libraries almost always do weird things behind your back. Libc will open files for looking up error messages or other locale dependent stuff, the resolver can open configuration files, etc. If you close a file descriptor and close it again, in a threaded environment you can easily end up in a situation where the file descriptor has been reused by a library and you close it behind its back.

Solution 3

Closing the same fd twice should be non-fatal as others have pointed out, but beware of code like this

close(fd);
/* ... */
newfd = open(....);
/* ... */
close(fd);

By the second close, you can't be sure if fd is actually the same as newfd! This would lead to crashes whenever you tried to use newfd.

So (if there's code between both close calls) it's not safe to do this. Always closefile descriptors exactly once. Always free buffers exactly once.

Solution 4

Second call will fail with Errno: EBADF when because by then, fd is not an active file descriptor.

It should have no effect at all on execution. However, if any error number was set by the first close, that will be lost, so you shouldn't close the file-descriptor twice.

Solution 5

If the value of fd remains the same the second call will return an error that the fd is not valid (EBADF - as dasblinkenlight pointed out)

Think of doing somthing likg

if fd != -1 )
{
   close (fd );
   fd = -1;
}
Share:
15,120

Related videos on Youtube

j10
Author by

j10

Updated on October 24, 2022

Comments

  • j10
    j10 over 1 year

    I just want to know what is the behavior of having two consecutive close on a fd.

    e.g.-

    close(fd);
    close(fd);
    

    [fd is an int]

  • Kerrek SB
    Kerrek SB over 11 years
    Why should you prevent the second call from happening?
  • Jim Balter
    Jim Balter over 11 years
    Setting fd to -1 has no effect (it certainly doesn't "prevent the second call from happening") ... both that and a closed fd yield EBADF.
  • Jim Balter
    Jim Balter over 11 years
    This sort of thing is pointless, since close(fd) isn't undefined or in any way dangerous when fd is already closed ... close(fd) and close(-1) do exactly the same thing. The only reason to set fd to -1 is to make sure it isn't a valid open file descriptor such as 0.
  • simonc
    simonc over 11 years
    @JimBalter setting fd to -1 would avoid the risk of very occasionally closing a file another thread had just opened.
  • Jim Balter
    Jim Balter over 11 years
    The first call to close might close an fd from another thread. Two closes aren't relevant to that issue. The only way threading is relevant here is that the second one might not fail after all if that fd was reopened, but that's uninteresting ... if you don't synchronize your threads them all sorts of bad things can happen.
  • Jim Balter
    Jim Balter over 11 years
    Ah, this answer actually says something valid! +1
  • Jim Balter
    Jim Balter over 11 years
    @simonc But if you don't synchronize your threads your code is full of race conditions and that's the least of your worries.
  • Sergey Kalinichenko
    Sergey Kalinichenko over 11 years
    @JimBalter Ah, of course you're right! I should have clarified that -1 by itself wouldn't prevent the call from happening. It would let you prevent the second call with an if statement, but it is useless all by itself (you'd get EBADF anyway). I edited the answer to include some code, and hope that it would eliminate the confusion. Thanks for the note!
  • Jim Balter
    Jim Balter over 11 years
    @simonc Ok, I've rethought that. Yes, you're right, but the real problem is that you're closing an fd without a corresponding open, and there's no reason to do that. But I suppose that setting the fd to -1 defends against a race condition that would be very hard to find if you did have that bug. So I take it all back.
  • Jim Balter
    Jim Balter over 11 years
    I think this answer should clarify that, if you need to close a file if it is open but don't necessarily know whether it is open, then you should set the fd to -1 ... but then there's no need to check whether it's -1, other than clarity ... it's ok to close(fd) regardless.
  • alk
    alk over 11 years
    The value returned by open() is recycled by the OS. So if thread A called open() and got fd=3 then close()s it. Then just before thread A calls close() the second time, thread B called open() successfully and got fd=3 again. So what would the second close() called by thread A do?
  • Jim Balter
    Jim Balter over 11 years
    I agree with the edit and retract my previous comment. Thanks for clarifying. But it's somewhat moot because the question is contrived -- there's no point in doing the second close.
  • Art
    Art over 11 years
    @JimBalter The other threads can do completely harmless things behind your back and you'll suddenly close file descriptors that they own. I edited the answer to explain a bit better.
  • Jim Balter
    Jim Balter over 11 years
    Ok, people have noted that a file descriptor may have been reopened in another thread, so setting fd = -1 is good practice. But fd != -1 test isn't strictly necessary ... the effect is the same without it.
  • alk
    alk over 11 years
    I do agree with you that the test for -1 is not needed. Even more: in a debug-build I'd omit it, and log the error close() would return then, to point me to the mess I created that lead to try closing the file twice ... ;-) @JimBalter
  • Art
    Art over 11 years
    This example is dangerous. All operating systems that I've explored this on (BSDs, Linux, Solaris) will close the file descriptor even when the internal part of close fails (close failing has been added because of file systems like AFS with sync on close semantics where you can lose your credentials between write and close). Doing the second close will close a file descriptor that has potentially been reused by someone else. Don't be smart with close, just close once and notify the user that something went wrong (which is what emacs does).
  • Jim Balter
    Jim Balter over 11 years
    @alk Agreed, unless you don't know whether the file is open and the close is conditional on whether it is ... then you would be logging an error that isn't one, and the test against -1 is necessary after all!
  • Art
    Art over 11 years
    To expand on my previous comment - POSIX says that if close returns an error "the state of fildes is unspecified". In the environments where I've looked at this the file descriptor will always be closed and reused.
  • Art
    Art over 11 years
    @dasblinkenlight Cool. I retract my objection then. I'll leave the comment just in case someone ever thinks about acting on the error return of close.
  • R.. GitHub STOP HELPING ICE
    R.. GitHub STOP HELPING ICE over 11 years
    The importance of this issue cannot be stressed enough. In a multi-threaded program, double-close errors can lead to serious data corruption, data compromise, information leaks, and even in some cases privilege elevation.
  • Mario The Spoon
    Mario The Spoon over 11 years
    @Jim agreed, but this comes from developing in a very stric, error avoiding environment for too long :-(
  • j10
    j10 over 11 years
    thank you guys for your explanation. I am not in a multi-threaded environment ... but the problem is i do not know if that fd has been closed somewhere and so i may end up doing close () twice ... so what i understand is that two close for me won't matter much because the time between two close is 2-3 instructions ......
  • Art
    Art over 11 years
    Just to add to the cosmic amusement, me and a coworker just spent 5 hours debugging a problem that turned out to be a double close in a threaded server. Sometimes reality catches up to theoretical arguments faster than you'd imagine.