RE: cvs commit: apr/file_io/win32 filedup.c open.c readwrite.c

[William A. Rowe, Jr.]

At 06:00 AM 10/29/2002, Bill Stoddard wrote:

>> And it has a flaw if we are logging errors from the parent, the old
>> race condition still remains.  It gets much worse if we factor in
>> the possibility of two child worker processes accessing the file
>> at once.  Thread locks are strictly intraprocess.
>
>The LockFile calls will protect against cross process updates to the file 
>(when in APR_APPEND mode). The thread locks are to work around what
>appears to be a bug in the Windows LockFile implementation. LockFile 
>can deadlock with WriteFile and I expect it is only an issue when multiple
> threads in a single process are accessing LockFile.

I wasn't reading closely enough, shame on me :-)

I see no problem with ignoring the pOverlapped when choosing to lock
the file, and it makes the code simpler.  

I'll commit the following patch a bit later today, unless you know 
of some restriction that I'm unaware of.

Bill

--- srclib/apr/file_io/win32/readwrite.c        29 Oct 2002 02:19:50 -0000      
1.71
+++ srclib/apr/file_io/win32/readwrite.c        29 Oct 2002 17:12:13 -0000
@@ -308,12 +308,10 @@
             apr_status_t rc;
             if (thefile->append) {
                 apr_thread_mutex_lock(thefile->mutex);
-                if (!thefile->pOverlapped) {
-                    rc = apr_file_lock(thefile, APR_FLOCK_EXCLUSIVE);
-                    if (rc != APR_SUCCESS) {
-                        apr_thread_mutex_unlock(thefile->mutex);
-                        return rc;
-                    }
+                rc = apr_file_lock(thefile, APR_FLOCK_EXCLUSIVE);
+                if (rc != APR_SUCCESS) {
+                    apr_thread_mutex_unlock(thefile->mutex);
+                    return rc;
                 }
                 rc = apr_file_seek(thefile, APR_END, &offset);
                 if (rc != APR_SUCCESS) {
@@ -328,9 +326,7 @@
             rv = WriteFile(thefile->filehand, buf, *nbytes, &bwrote,
                            thefile->pOverlapped);
             if (thefile->append) {
-                if (!thefile->pOverlapped) {
-                    apr_file_unlock(thefile);
-                }
+                apr_file_unlock(thefile);
                 apr_thread_mutex_unlock(thefile->mutex);
             }
         }