POSIX close(2) is broken

Re:POSIX close(2) is broken by 1amzave on Saturday December 17, 2011 @05:00AM (#3363934)

My first reaction to the thread-safety problem: fstat before close, if it fails with EINTR, fstat again. Then you can tell both the state of the fd (since fstat will return EBADF if it's closed), and if it's still a valid fd, what file it actually refers to. So if another thread did open() and reallocate the same fd number, you'll get a different struct stat back, and then know not to retry the close.

As in:

  int safeclose(int fd)
  {
  	struct stat before, after;
  
  	if (fstat(fd,&before))
  		return -1;
  
  	while (close(fd)) {
  		if (errno != EINTR)
  			return -1;
  
  		/* If fstat() fails, our close() succeeded */
  		if (fstat(fd,&after))
  			return 0;
  
  		/* If we've got a different file, our close() succeeded */
  		if (before.st_dev != after.st_dev
  		    || before.st_ino != after.st_ino /* whatever other necessary checks here... */)
  			return 0;
  		/* Otherwise we've got the same file still open, retry */
  	}
  
  	return 0;		
  }

Any problems/races I'm not seeing? Is comparing 'struct stat' potentially not 100% reliable?

[ Parent | on HN ]

Re:POSIX close(2) is broken by philh on Saturday December 17, 2011 @05:12AM (#3363947)

It looks like if the other thread opened the same file you might get a false positive.

    struct stat {
        dev_t     st_dev;     /* ID of device containing file */
        ino_t     st_ino;     /* inode number */
        mode_t    st_mode;    /* protection */
        nlink_t   st_nlink;   /* number of hard links */
        uid_t     st_uid;     /* user ID of owner */
        gid_t     st_gid;     /* group ID of owner */
        dev_t     st_rdev;    /* device ID (if special file) */
        off_t     st_size;    /* total size, in bytes */
        blksize_t st_blksize; /* blocksize for file system I/O */
        blkcnt_t  st_blocks;  /* number of 512B blocks allocated */
        time_t    st_atime;   /* time of last access */
        time_t    st_mtime;   /* time of last modification */
        time_t    st_ctime;   /* time of last status change */
    };

The only one of these likely to change is st_atime, but I don't think that's guaranteed to update. (I think different behaviours can be compiled into the linux kernel, including "only update atime if it equals ctime or mtime".)

[ Parent | on HN ]

Re:POSIX close(2) is broken by justincormack on Saturday December 17, 2011 @06:10AM (#3364016)

Or mounting a filesystem with the noatime option, which is common.

[ Parent | on HN ]

Re:POSIX close(2) is broken by FrankBooth on Saturday December 17, 2011 @05:07AM (#3363942)

What if the other thread opens the same file?

[ Parent | on HN ]

Re:POSIX close(2) is broken by 1amzave on Saturday December 17, 2011 @05:13AM (#3363948)

True, hadn't thought of that. (Seems obvious in retrospect, of course.)

It's almost tempting to try to differentiate that with st_atime, but I don't think there's any way that could be reliable (especially given noatime mounts and such).

[ Parent | on HN ]

Re:POSIX close(2) is broken by nknight on Saturday December 17, 2011 @05:14AM (#3363949)

Three obvious problems, actually:

1) Thread A closes fd X for File Y, Thread B opens fd X for File Y. Thread A sees fd X referring to File Y, closes it again out from under Thread B.

2) Since the behavior is unspecified, it's possible you could have fd X briefly continue to point to File Y. This could lead to a case where your fstat() results cease to be valid by the time you run close() again.

3) fd doesn't have to be a normal on-disk file, and POSIX is somewhat vague about what the stat struct's values are for the various possible types of "file".

[ Parent | on HN ]