apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject RE: cvs commit: apr/file_io/win32 filedup.c open.c readwrite.c
Date Tue, 29 Oct 2002 17:32:44 GMT

> 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 :-)

Hey, I've never been guilty of that! Keep on your toes boy. ;-) Seriously, I am
really glad you are reviewing this code. Thanks.

>
> 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

The reason for locking the file is to mutex the file pointer. A file opened for
overlapped i/o does not have a file pointer.
I'm not aware of any restrictions but I've not tested this patch and I think the
results will be less than predictable. For instance, we lock the file, call
WriteFile and get back IO_PENDING, then we unlock the file. Maybe the i/o
occured by the time we released the lock. Or maybe not.  The function is broken,
with or without this patch when used for writing to files opened for overlapped
i/o. I'm not sure the best way to fix it (or even if we should fix it). I'm -0
on this patch.

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);
>              }
>          }
>


Mime
View raw message