diff options
author | Ivan Zhakov <ivan@apache.org> | 2017-08-29 15:14:44 +0000 |
---|---|---|
committer | Ivan Zhakov <ivan@apache.org> | 2017-08-29 15:14:44 +0000 |
commit | 9d77ad02f84e2a9089d83d793a00e664cdab1854 (patch) | |
tree | 11bbf066143f5d894b4ee8bd1fde59ac301e649d /CHANGES | |
parent | 908eeba5cc16bde7bb392af1168d9db7df4ff515 (diff) | |
download | apr-9d77ad02f84e2a9089d83d793a00e664cdab1854.tar.gz |
Win32: Fix a deadlock when appending to locked files (PR50058).
Currently, appending data to a file opened with APR_FOPEN_APPEND
that has been locked by apr_file_lock() will cause a deadlock on Windows.
[See PR50058, https://bz.apache.org/bugzilla/show_bug.cgi?id=50058]
This issue happens because atomic O_APPEND-style appends on Windows
are implemented using file locks. An append happens while holding the
file lock acquired with LockFile(), which is required to avoid a race
condition between seeking to the end of file and writing data. The
race is possible when multiple threads or processes are appending
data to the same file.
This approach causes a deadlock if the file has been previously locked
with apr_file_lock(). (Note that it's perfectly legit to lock the file or
its portion and perform the append after that.)
Apart from this, using file locks for file appends impacts their speed and
robustness. There is an overhead associated with locking the file, especially
if the file is not local. The robustness is affected, because other writes
to the same file may fail due to it being locked. Also, if a process is
terminated in the middle of the append operation, it might take some time
for the OS to release the file lock. During this time, the file would be
inaccessible to other processes. This may affect applications such as
httpd (with a multi-process MPM) that use APR_FOPEN_APPEND files
for logging.
This patch fixes the issue by switching to the documented way to
atomically append data with a single WriteFile() call. It requires passing
special OVERLAPPED.Offset and OffsetHigh values (0xFFFFFFFF). On the
ZwWriteFile() layer, this maps to a FILE_WRITE_TO_END_OF_FILE constant
that instructs the OS (the corresponding file system driver) to write data
to the file's end.
Note that this approach is only used for files opened for synchronous I/O
because in this case the I/O Manager maintains the current file position.
Otherwise, the file offset returned or changed by the SetFilePointer() API is
not guaranteed to be valid and that could, for instance, break apr_file_seek()
calls after appending data. Sadly, if a file is opened for asynchronous I/O,
this call to WriteFile() doesn't update the OVERLAPPED.Offset member to
reflect the actual offset used when appending the data (which we could
then use to make seeking and other operations involving filePtr work).
Therefore, when appending to files opened for asynchronous I/O, we still
use the old LockFile + SetFilePointer + WriteFile approach.
Additional details on this can be found in:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365747
https://msdn.microsoft.com/en-us/library/windows/hardware/ff567121
* file_io/win32/readwrite.c
(apr_file_write): For files opened for synchronous I/O, use the
documented way to perform an atomic append with a single WriteFile().
* test/testfile.c
(): Include apr_thread_proc.h and apr_strings_.h
(struct thread_file_append_ctx_t, thread_file_append_func): New
test helpers.
(test_atomic_append, test_append_locked): New tests.
(testfile): Run the new tests.
* CHANGES: Add changelog entry.
Patch by: Evgeny Kotkov <evgeny.kotkov {at} visualsvn.com>
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1806608 13f79535-47bb-0310-9956-ffa450edef68
Diffstat (limited to 'CHANGES')
-rw-r--r-- | CHANGES | 3 |
1 files changed, 3 insertions, 0 deletions
@@ -1,5 +1,8 @@ -*- coding: utf-8 -*- Changes for APR 2.0.0 + *) Fix a deadlock when writing to locked files opened with APR_FOPEN_APPEND + on Windows. PR 50058. + [Evgeny Kotkov <evgeny.kotkov visualsvn.com>] *) apr_file_trunc: Truncating a buffered file could add unexpected data after the truncate position. PR 51017. |