You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ivan Zhakov <iv...@visualsvn.com> on 2013/08/23 22:33:38 UTC

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

[resurrecting this very old thread, see my reply below]

On Tue, Apr 27, 2010 at 3:03 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Tue, Apr 27, 2010 at 5:47 AM, Bert Huijben <be...@qqmail.nl> wrote:
[...]

>>> On Mon, Apr 26, 2010 at 4:38 PM, William A. Rowe Jr.
>>> <wr...@rowe-clan.net> wrote:
>>> > On 4/26/2010 2:19 PM, Jeff Trawick wrote:
>>> >>
>>> >> So I don't think there's any hidden "reason" why a mutex should always
>>> >> be obtained on Windows.  I too wouldn't be surprised if the fix breaks
>>> >> some app code somewhere.
>>> >
>>> > Keep in mind fd-based operations are atomic on Unix, but not so on
>>> windows.
>>>
>>> Since these are buffered files, it doesn't even come down to
>>> differences in OS file operations; operations on the buffer would be
>>> the expected failure point.
>>>
>>> So the question is whether or not APR expects multi-threaded apps
>>> sharing a buffered file to turn on the XTHREAD flag.
>>
>> Another thing I was thinking about is how the append mode is used. I can
>> imagine that is used to write to a single logfile in a multithreaded
>> application. (But if we don't enable the mutex on other operating systems,
>> it should probably be fixed in the application)
>
> Append is special, in that POSIX has an append flag on open() which
> handles atomicity of seek-to-end+write; so that mutex isn't needed on
> all operating systems.  This is a case where Windows needs a mutex but
> Unix doesn't.
Actually Windows supports atomic seek-to-end+write: file should be
opened with FILE_APPEND_DATA access right only [1] or Offset and
OffsetHigh should be 0xFFFFFFFF if overlapped I/O is used [2].

I'm reopening this thread because in Subversion we found case where we
need true atomic append across processes/threads. So I'm willing to
create a patch implementing atomic append on Windows. Is right idea
for APR or not? Any concerns will be very helpful.

[1] http://msdn.microsoft.com/en-us/library/windows/desktop/aa363778%28v=vs.85%29.aspx
[2] http://msdn.microsoft.com/en-us/library/windows/desktop/aa365747%28v=vs.85%29.aspx

-- 
Ivan Zhakov

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Saturday, August 24, 2013, Branko Čibej wrote:

> On 24.08.2013 06:29, Ivan Zhakov wrote:
> > On Sat, Aug 24, 2013 at 3:48 AM, William A. Rowe Jr.
> > <wrowe@rowe-clan.net <javascript:;>> wrote:
> >> On Fri, 23 Aug 2013 18:39:35 -0500
> >> "William A. Rowe Jr." <wrowe@rowe-clan.net <javascript:;>> wrote:
> >>
> >>> On Sat, 24 Aug 2013 00:33:38 +0400
> >>> Ivan Zhakov <ivan@visualsvn.com <javascript:;>> wrote:
> >>>
> >>>> Actually Windows supports atomic seek-to-end+write: file should be
> >>>> opened with FILE_APPEND_DATA access right only [1] or Offset and
> >>>> OffsetHigh should be 0xFFFFFFFF if overlapped I/O is used [2].
> >>>>
> >>>> I'm reopening this thread because in Subversion we found case where
> >>>> we need true atomic append across processes/threads. So I'm willing
> >>>> to create a patch implementing atomic append on Windows. Is right
> >>>> idea for APR or not? Any concerns will be very helpful.
> >>>>
> >>>> [1]
> >>>>
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa363778%28v=vs.85%29.aspx
> >>>> [2]
> >>>>
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa365747%28v=vs.85%29.aspx
> >>> IIRC the difference is that you have writev on unix to atomically
> >>> write multiple buffers.  On Windows we fake writev, so your proposed
> >>> atomic writes are no longer atomic.
> > Subversion doesn't use writev for file I/O, so implementing atomic
> > writes is enough for our case.
>
> Would that mean that writev on Windows uses a mutex while plain write
> does not? How do you avoid a race between write and writev then?

Currently apr_file_writev is alias for calling apr_file_write multiple
times without taking lock. Also please note that mutexes helps only if
write performed using same file_t in one process, it does not race
conditions when write operation from different threads or processes using
different handles.


-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by Branko Čibej <br...@apache.org>.
On 24.08.2013 06:29, Ivan Zhakov wrote:
> On Sat, Aug 24, 2013 at 3:48 AM, William A. Rowe Jr.
> <wr...@rowe-clan.net> wrote:
>> On Fri, 23 Aug 2013 18:39:35 -0500
>> "William A. Rowe Jr." <wr...@rowe-clan.net> wrote:
>>
>>> On Sat, 24 Aug 2013 00:33:38 +0400
>>> Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>
>>>> Actually Windows supports atomic seek-to-end+write: file should be
>>>> opened with FILE_APPEND_DATA access right only [1] or Offset and
>>>> OffsetHigh should be 0xFFFFFFFF if overlapped I/O is used [2].
>>>>
>>>> I'm reopening this thread because in Subversion we found case where
>>>> we need true atomic append across processes/threads. So I'm willing
>>>> to create a patch implementing atomic append on Windows. Is right
>>>> idea for APR or not? Any concerns will be very helpful.
>>>>
>>>> [1]
>>>> http://msdn.microsoft.com/en-us/library/windows/desktop/aa363778%28v=vs.85%29.aspx
>>>> [2]
>>>> http://msdn.microsoft.com/en-us/library/windows/desktop/aa365747%28v=vs.85%29.aspx
>>> IIRC the difference is that you have writev on unix to atomically
>>> write multiple buffers.  On Windows we fake writev, so your proposed
>>> atomic writes are no longer atomic.
> Subversion doesn't use writev for file I/O, so implementing atomic
> writes is enough for our case.

Would that mean that writev on Windows uses a mutex while plain write
does not? How do you avoid a race between write and writev then?

-- Brane


Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, Aug 24, 2013 at 3:48 AM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> On Fri, 23 Aug 2013 18:39:35 -0500
> "William A. Rowe Jr." <wr...@rowe-clan.net> wrote:
>
>> On Sat, 24 Aug 2013 00:33:38 +0400
>> Ivan Zhakov <iv...@visualsvn.com> wrote:
>>
>> > Actually Windows supports atomic seek-to-end+write: file should be
>> > opened with FILE_APPEND_DATA access right only [1] or Offset and
>> > OffsetHigh should be 0xFFFFFFFF if overlapped I/O is used [2].
>> >
>> > I'm reopening this thread because in Subversion we found case where
>> > we need true atomic append across processes/threads. So I'm willing
>> > to create a patch implementing atomic append on Windows. Is right
>> > idea for APR or not? Any concerns will be very helpful.
>> >
>> > [1]
>> > http://msdn.microsoft.com/en-us/library/windows/desktop/aa363778%28v=vs.85%29.aspx
>> > [2]
>> > http://msdn.microsoft.com/en-us/library/windows/desktop/aa365747%28v=vs.85%29.aspx
>>
>> IIRC the difference is that you have writev on unix to atomically
>> write multiple buffers.  On Windows we fake writev, so your proposed
>> atomic writes are no longer atomic.
>
Subversion doesn't use writev for file I/O, so implementing atomic
writes is enough for our case.

> Now we might use WriteFileGather at this point (remember, this stuff
> was all written when Server 2000 was still around :)... but note that
> the compatibility is listed as "Windows Server 2003 [Desktop apps only].
> WTF would that mean?  Also not supported for 32 bit Itanium apps running
> under WOW64.  How bizarre.  Then read the description of the
> aSegmentArray arg to really break your brain.
>
WriteFileGather does not help anyway: provided buffers must be at
least the size of a system memory page and must be aligned on a system
memory page size boundary.



-- 
Ivan Zhakov

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On Fri, 23 Aug 2013 18:39:35 -0500
"William A. Rowe Jr." <wr...@rowe-clan.net> wrote:

> On Sat, 24 Aug 2013 00:33:38 +0400
> Ivan Zhakov <iv...@visualsvn.com> wrote:
> 
> > Actually Windows supports atomic seek-to-end+write: file should be
> > opened with FILE_APPEND_DATA access right only [1] or Offset and
> > OffsetHigh should be 0xFFFFFFFF if overlapped I/O is used [2].
> > 
> > I'm reopening this thread because in Subversion we found case where
> > we need true atomic append across processes/threads. So I'm willing
> > to create a patch implementing atomic append on Windows. Is right
> > idea for APR or not? Any concerns will be very helpful.
> > 
> > [1]
> > http://msdn.microsoft.com/en-us/library/windows/desktop/aa363778%28v=vs.85%29.aspx
> > [2]
> > http://msdn.microsoft.com/en-us/library/windows/desktop/aa365747%28v=vs.85%29.aspx
> 
> IIRC the difference is that you have writev on unix to atomically 
> write multiple buffers.  On Windows we fake writev, so your proposed
> atomic writes are no longer atomic.

Now we might use WriteFileGather at this point (remember, this stuff
was all written when Server 2000 was still around :)... but note that
the compatibility is listed as "Windows Server 2003 [Desktop apps only].
WTF would that mean?  Also not supported for 32 bit Itanium apps running
under WOW64.  How bizarre.  Then read the description of the
aSegmentArray arg to really break your brain.





Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On Sat, 24 Aug 2013 00:33:38 +0400
Ivan Zhakov <iv...@visualsvn.com> wrote:

> [resurrecting this very old thread, see my reply below]
> 
> On Tue, Apr 27, 2010 at 3:03 PM, Jeff Trawick <tr...@gmail.com>
> wrote:
> > On Tue, Apr 27, 2010 at 5:47 AM, Bert Huijben <be...@qqmail.nl>
> > wrote:
> [...]
> 
> >>> On Mon, Apr 26, 2010 at 4:38 PM, William A. Rowe Jr.
> >>> <wr...@rowe-clan.net> wrote:
> >>> > On 4/26/2010 2:19 PM, Jeff Trawick wrote:
> >>> >>
> >>> >> So I don't think there's any hidden "reason" why a mutex
> >>> >> should always be obtained on Windows.  I too wouldn't be
> >>> >> surprised if the fix breaks some app code somewhere.
> >>> >
> >>> > Keep in mind fd-based operations are atomic on Unix, but not so
> >>> > on
> >>> windows.
> >>>
> >>> Since these are buffered files, it doesn't even come down to
> >>> differences in OS file operations; operations on the buffer would
> >>> be the expected failure point.
> >>>
> >>> So the question is whether or not APR expects multi-threaded apps
> >>> sharing a buffered file to turn on the XTHREAD flag.
> >>
> >> Another thing I was thinking about is how the append mode is used.
> >> I can imagine that is used to write to a single logfile in a
> >> multithreaded application. (But if we don't enable the mutex on
> >> other operating systems, it should probably be fixed in the
> >> application)
> >
> > Append is special, in that POSIX has an append flag on open() which
> > handles atomicity of seek-to-end+write; so that mutex isn't needed
> > on all operating systems.  This is a case where Windows needs a
> > mutex but Unix doesn't.
> Actually Windows supports atomic seek-to-end+write: file should be
> opened with FILE_APPEND_DATA access right only [1] or Offset and
> OffsetHigh should be 0xFFFFFFFF if overlapped I/O is used [2].
> 
> I'm reopening this thread because in Subversion we found case where we
> need true atomic append across processes/threads. So I'm willing to
> create a patch implementing atomic append on Windows. Is right idea
> for APR or not? Any concerns will be very helpful.
> 
> [1]
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa363778%28v=vs.85%29.aspx
> [2]
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa365747%28v=vs.85%29.aspx

IIRC the difference is that you have writev on unix to atomically 
write multiple buffers.  On Windows we fake writev, so your proposed
atomic writes are no longer atomic.

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 26 August 2013 at 20:13, Stefan Ruppert <sr...@myarm.com> wrote:
> Am 26.08.2013 17:48, schrieb Ivan Zhakov:
>>
>> On Mon, Aug 26, 2013 at 7:41 PM, William A. Rowe Jr.
>> <wr...@rowe-clan.net> wrote:
>>>
>>> On Mon, 26 Aug 2013 18:58:21 +0400
>>> Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>
>>>> Alternative solution would be take file->lock mutex on write operation
>>>> and user shared OVERLAPPED structure. Also note that MSDN states that
>>>> opening file with FILE_APPEND permissions only will perform atomic
>>>> move-to-end + write without overlapped I/O.
>>>
>>>
>>> Sure, that works for files, but apr_file_write is not strictly for
>>> true file-based handles, right?
>>>
>> Sure, but I think APR_FOPEN_OPEN does makes sense only for files. And
>> we have file->append flag to check if file opened in append mode.
>>
>>> Some general thoughts... until apr_file_writev is called, we won't
>>> need the append file lock.  But... we need to mutex against the
>>> parallel calls to apr_file_writev so we can't create the mutex to
>>> interlock at that moment.
>>>
>> Note that this doesn't solve problem with several processes appending
>> to same file.
>
>
> Yes, I encounterd this problem three years ago and filed a bug:
> https://issues.apache.org/bugzilla/show_bug.cgi?id=50058
>
> If you open a file in append mode under Windows and use your own file
> locking calling the following sequence:
>
> apr_file_open(FOPEN_APPEND);
> apr_file_lock();
> apr_file_write();
> apr_file_write();
> apr_file_write();
> apr_file_unlock();
> apr_file_close();
>
> It will deadlock under Windows. Under Linux it works fine.
>
> For my usage scenario an implementation of apr_file_lock()/apr_file_unlock()
> which supports nested calls would work. Then apr_file_writev() could also
> use a apr_file_lock()/apr_file_unlock() pair. What do you think?
>
Just for the records: the problem described above has been fixed in r1806608 [1]

https://svn.apache.org/r1806608

-- 
Ivan Zhakov

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by Stefan Ruppert <sr...@myarm.com>.
Am 26.08.2013 17:48, schrieb Ivan Zhakov:
> On Mon, Aug 26, 2013 at 7:41 PM, William A. Rowe Jr.
> <wr...@rowe-clan.net> wrote:
>> On Mon, 26 Aug 2013 18:58:21 +0400
>> Ivan Zhakov <iv...@visualsvn.com> wrote:
>>
>>> Alternative solution would be take file->lock mutex on write operation
>>> and user shared OVERLAPPED structure. Also note that MSDN states that
>>> opening file with FILE_APPEND permissions only will perform atomic
>>> move-to-end + write without overlapped I/O.
>>
>> Sure, that works for files, but apr_file_write is not strictly for
>> true file-based handles, right?
>>
> Sure, but I think APR_FOPEN_OPEN does makes sense only for files. And
> we have file->append flag to check if file opened in append mode.
>
>> Some general thoughts... until apr_file_writev is called, we won't
>> need the append file lock.  But... we need to mutex against the
>> parallel calls to apr_file_writev so we can't create the mutex to
>> interlock at that moment.
>>
> Note that this doesn't solve problem with several processes appending
> to same file.

Yes, I encounterd this problem three years ago and filed a bug:
https://issues.apache.org/bugzilla/show_bug.cgi?id=50058

If you open a file in append mode under Windows and use your own file 
locking calling the following sequence:

apr_file_open(FOPEN_APPEND);
apr_file_lock();
apr_file_write();
apr_file_write();
apr_file_write();
apr_file_unlock();
apr_file_close();

It will deadlock under Windows. Under Linux it works fine.

For my usage scenario an implementation of 
apr_file_lock()/apr_file_unlock() which supports nested calls would 
work. Then apr_file_writev() could also use a 
apr_file_lock()/apr_file_unlock() pair. What do you think?

Stefan


Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Mon, Aug 26, 2013 at 7:41 PM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> On Mon, 26 Aug 2013 18:58:21 +0400
> Ivan Zhakov <iv...@visualsvn.com> wrote:
>
>> Alternative solution would be take file->lock mutex on write operation
>> and user shared OVERLAPPED structure. Also note that MSDN states that
>> opening file with FILE_APPEND permissions only will perform atomic
>> move-to-end + write without overlapped I/O.
>
> Sure, that works for files, but apr_file_write is not strictly for
> true file-based handles, right?
>
Sure, but I think APR_FOPEN_OPEN does makes sense only for files. And
we have file->append flag to check if file opened in append mode.

> Some general thoughts... until apr_file_writev is called, we won't
> need the append file lock.  But... we need to mutex against the
> parallel calls to apr_file_writev so we can't create the mutex to
> interlock at that moment.
>
Note that this doesn't solve problem with several processes appending
to same file.



-- 
Ivan Zhakov

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On Mon, 26 Aug 2013 18:58:21 +0400
Ivan Zhakov <iv...@visualsvn.com> wrote:

> Alternative solution would be take file->lock mutex on write operation
> and user shared OVERLAPPED structure. Also note that MSDN states that
> opening file with FILE_APPEND permissions only will perform atomic
> move-to-end + write without overlapped I/O.

Sure, that works for files, but apr_file_write is not strictly for
true file-based handles, right?

Some general thoughts... until apr_file_writev is called, we won't
need the append file lock.  But... we need to mutex against the
parallel calls to apr_file_writev so we can't create the mutex to
interlock at that moment.

... we need completion contexts with an event-per-thread ... but
these do not span apr_ calls.  What about parking a per-thread
apr-shared completion context created on apr thread startup and
the event handle closed on apr thread teardown?


Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Mon, Aug 26, 2013 at 6:43 PM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> On Sat, 24 Aug 2013 00:33:38 +0400
> Ivan Zhakov <iv...@visualsvn.com> wrote:
>> >
>> > Append is special, in that POSIX has an append flag on open() which
>> > handles atomicity of seek-to-end+write; so that mutex isn't needed
>> > on all operating systems.  This is a case where Windows needs a
>> > mutex but Unix doesn't.
>> Actually Windows supports atomic seek-to-end+write: file should be
>> opened with FILE_APPEND_DATA access right only [1] or Offset and
>> OffsetHigh should be 0xFFFFFFFF if overlapped I/O is used [2].
>>
>> I'm reopening this thread because in Subversion we found case where we
>> need true atomic append across processes/threads. So I'm willing to
>> create a patch implementing atomic append on Windows. Is right idea
>> for APR or not? Any concerns will be very helpful.
>>
>> [1]
>> http://msdn.microsoft.com/en-us/library/windows/desktop/aa363778%28v=vs.85%29.aspx
>> [2]
>> http://msdn.microsoft.com/en-us/library/windows/desktop/aa365747%28v=vs.85%29.aspx
>
> This is even worse than I thought...
>
> 1. we aren't protecting segments of writev within the apr_file_writev
>    call with the mutex (as Ivan notes)
>
> 2. the mutex is used for buffered output (which makes sense, if
>    you have multiple threads with the same apr_file_t context which
>    write to the same buffer...)
>
> 3. The mutex -> file lock looks backwards and we release the mutex...
>    it is a complete mess.
>
> 4. If the file lock doesn't support nested locks (have yet to check)
>    then this whole code path is foobar for anyone using the file lock
>    API within their own code (as would be appropriate for complex
>    write operations).
>
> 5. On failure we use pOverlapped with a thread-shared overlapped IO
>    structure and hEvent signal.  This is not unique per-thread and is
>    clearly a design error if we pass this without holding the mutex.
>
> I need to go a decade back in svn to pick up the initial principals
> and we need to recode both write and writev appropriately.
>
> If opened for append, unbuffered, non-xthread then I can see us finding
> our way to the atomic write-for-append method on unix and windows.  But
> that method will force us to create the overlapped and hEvent on each
> call to write to ensure they are unique per-thread, slowing things down
> considerably, in the write failure case.
Alternative solution would be take file->lock mutex on write operation
and user shared OVERLAPPED structure. Also note that MSDN states that
opening file with FILE_APPEND permissions only will perform atomic
move-to-end + write without overlapped I/O.


-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On Sat, 24 Aug 2013 00:33:38 +0400
Ivan Zhakov <iv...@visualsvn.com> wrote:
> >
> > Append is special, in that POSIX has an append flag on open() which
> > handles atomicity of seek-to-end+write; so that mutex isn't needed
> > on all operating systems.  This is a case where Windows needs a
> > mutex but Unix doesn't.
> Actually Windows supports atomic seek-to-end+write: file should be
> opened with FILE_APPEND_DATA access right only [1] or Offset and
> OffsetHigh should be 0xFFFFFFFF if overlapped I/O is used [2].
> 
> I'm reopening this thread because in Subversion we found case where we
> need true atomic append across processes/threads. So I'm willing to
> create a patch implementing atomic append on Windows. Is right idea
> for APR or not? Any concerns will be very helpful.
> 
> [1]
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa363778%28v=vs.85%29.aspx
> [2]
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa365747%28v=vs.85%29.aspx

This is even worse than I thought...

1. we aren't protecting segments of writev within the apr_file_writev
   call with the mutex (as Ivan notes)

2. the mutex is used for buffered output (which makes sense, if
   you have multiple threads with the same apr_file_t context which
   write to the same buffer...)

3. The mutex -> file lock looks backwards and we release the mutex...
   it is a complete mess.

4. If the file lock doesn't support nested locks (have yet to check)
   then this whole code path is foobar for anyone using the file lock
   API within their own code (as would be appropriate for complex
   write operations).

5. On failure we use pOverlapped with a thread-shared overlapped IO
   structure and hEvent signal.  This is not unique per-thread and is
   clearly a design error if we pass this without holding the mutex.

I need to go a decade back in svn to pick up the initial principals
and we need to recode both write and writev appropriately.

If opened for append, unbuffered, non-xthread then I can see us finding
our way to the atomic write-for-append method on unix and windows.  But
that method will force us to create the overlapped and hEvent on each
call to write to ensure they are unique per-thread, slowing things down
considerably, in the write failure case.