At 09:27 AM 10/25/2002, Bill Stoddard wrote:
>The only problem with this patch [...]
>
>--- readwrite.c 19 Sep 2002 12:20:20 -0000 1.1.1.1
>+++ readwrite.c 25 Oct 2002 13:44:53 -0000
>- if (thefile->pOverlapped && !thefile->pipe) {
>- thefile->pOverlapped->Offset = (DWORD)thefile->filePtr;
>- thefile->pOverlapped->OffsetHigh = (DWORD)(thefile->filePtr >>
>32);
>- }
>- else if (!thefile->pipe && thefile->append) {
>+ if (thefile->append && !thefile->pipe) {
>+ int rc;
>+ rc = apr_file_lock(thefile, APR_FLOCK_EXCLUSIVE);
>+ if (rc != APR_SUCCESS) {
>+ rc = APR_SUCCESS;
>+ }
> SetFilePointer(thefile->filehand, 0, NULL, FILE_END);
You can't just drop the Overlapped test. there is NO CURRENT POSITION
if you have opened a file for overlapped I/O. Please don't assume that
someone won't mix the APR_APPEND bit with either APR_XTHREAD
or APR_SENDFILE_ENABLE (both of which must toggle APR_XTHREAD
so TransmitFile has an overlapped file.)
It seems you only need APR_FLOCK_WRITE in this context, we
really shouldn't lock users that are attempting to tail -f the file.
You also can't ignore the return value from the lock call.
> The only problem with this patch is that it occasionally causes the
> WriteFile to block indefinitely and all the other worker threads block on
> apr_file_lock waiting for the file lock. I can send several thousand
> requests through the server before it hangs. I don't see
> anything obviously
> wrong here or in apr_file_lock|unlock. Obviously I will not commit this
> until we figure out and fix what is causing the WriteFile block.
>
I suspect that's you ignoring rv != APR_SUCCESS...
Try putting this back together with the correct overlapped handling
and I'd be happy to apply it and work with if for a while.
Only one oddball concern, and that's networked files, which do very
odd things with file locking, sometimes.
Bill
