Double close possible in file_descriptor_impl::close_impl()
- Dominant language
- C
- Stars
- 48
- Forks
- 124
- PR merge metrics
- No merged PRs in 30d
Description
The current implementation of file_descriptor_impl::close_impl() will cause a double close on a file descriptor if you explicitly call close() and it results in an error (e.g. BOOST_IOSTREAMS_FD_CLOSE() returns a failure due to EIO or ENOSPC). In this case, the first call to close_impl() (from file_descriptor_impl::close()) will call the operating system's close() call on handle_ then throw an exception before it clears handle_ (by setting it to invalid_handle()). When the destructor is called, handle_ is still set to the old file handle, so the destructor will call the operating system's close() again. In the mean time, another thread may have opened that same file / socket handle. If you're lucky, that other thread will get some kind of IO error. If you're not lucky (as was my case), yet another thread then opens the same file handle (because it was closed again), at which point you have really weird file corruption.
Proposed patch:
diff --git a/src/file_descriptor.cpp b/src/file_descriptor.cpp
index 288e2c6..77dd65d 100644
```
--- a/src/file_descriptor.cpp
+++ b/src/file_descriptor.cpp
@@ -266,8 +266,11 @@ void file_descriptor_impl::close_impl(bool close_flag, bool throw_) {
#else
BOOST_IOSTREAMS_FD_CLOSE(handle_) != -1;
#endif
- if (!success && throw_)
+ if (!success && throw_) {
+ handle_ = invalid_handle();
+ flags_ = 0;
throw_system_failure("failed closing file");
+ }
}
handle_ = invalid_handle();
flags_ = 0;
```
I'm not sure about flags_, but handle should definitely be set to invalid_handle() before throwing the exception.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in src/file_descriptor.cpp at file_descriptor_impl::close_impl(), then trace the explicit close() path and destructor cleanup. Verify the failure path after BOOST_IOSTREAMS_FD_CLOSE() does not leave the handle available for a second close, and check whether flags_ should be cleared as well; done means the proposed regression is prevented without changing successful cleanup.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100