3 ms·
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 fsta
by 1amzave 15y ago
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?
- FrankBooth 15y agoWhat if the other thread opens the same file?
- 1amzave 15y agoTrue, 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).
- philh 15y agoIt 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".)
- justincormack 15y agoOr mounting a filesystem with the noatime option, which is common.
- nknight 15y agoThree 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".