public inbox for cygwin-developers@cygwin.com
 help / color / mirror / Atom feed
* thread safety of fhandler_base_overlapped
@ 2021-12-20 23:02 David McFarland
  2021-12-20 23:48 ` Ken Brown
  2021-12-20 23:58 ` Takashi Yano
  0 siblings, 2 replies; 5+ messages in thread
From: David McFarland @ 2021-12-20 23:02 UTC (permalink / raw)
  To: cygwin-developers; +Cc: corngood

I'm seeing a problem where occasionally write() returns EAGAIN on a
blocking pipe. I believe this happens when multiple threads are writing
concurrently to the pipe, and the overlapped structure is modified by
one thread while another one is between calls to WFMO and
GetOverlappedResult.

I found this change:

=====
commit 2a4b1de773e6159ea8197b6fc3f7e4845479b476
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Mon Mar 23 21:06:00 2020 +0100

    Cygwin: serial: use per call OVERLAPPED structs

    Sharing the OVERLAPPED struct and event object in there between
    read and select calls in the fhandler might have been a nice
    optimization way back when, but it is a dangerous, not thread-safe
    approach.  Fix this by creating per-fhandler, per-call OVERLAPPED
    structs and event objects on demand.

    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
=====

which seems to fix a similar problem in fhandler_serial.

Is is possible that this needs to be done for fhandler_base_overlapped
as well? I'll try using per-call buffers to see if it fixes my problem.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: thread safety of fhandler_base_overlapped
  2021-12-20 23:02 thread safety of fhandler_base_overlapped David McFarland
@ 2021-12-20 23:48 ` Ken Brown
  2021-12-20 23:58 ` Takashi Yano
  1 sibling, 0 replies; 5+ messages in thread
From: Ken Brown @ 2021-12-20 23:48 UTC (permalink / raw)
  To: cygwin-developers

On 12/20/2021 6:02 PM, David McFarland wrote:
> I'm seeing a problem where occasionally write() returns EAGAIN on a
> blocking pipe. I believe this happens when multiple threads are writing
> concurrently to the pipe, and the overlapped structure is modified by
> one thread while another one is between calls to WFMO and
> GetOverlappedResult.
> 
> I found this change:
> 
> =====
> commit 2a4b1de773e6159ea8197b6fc3f7e4845479b476
> Author: Corinna Vinschen <corinna@vinschen.de>
> Date:   Mon Mar 23 21:06:00 2020 +0100
> 
>      Cygwin: serial: use per call OVERLAPPED structs
> 
>      Sharing the OVERLAPPED struct and event object in there between
>      read and select calls in the fhandler might have been a nice
>      optimization way back when, but it is a dangerous, not thread-safe
>      approach.  Fix this by creating per-fhandler, per-call OVERLAPPED
>      structs and event objects on demand.
> 
>      Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
> =====
> 
> which seems to fix a similar problem in fhandler_serial.
> 
> Is is possible that this needs to be done for fhandler_base_overlapped
> as well? I'll try using per-call buffers to see if it fixes my problem.

fhandler_base_overlapped was removed in commit f002d02b17.  Are you looking at 
an old version of the sources?

Ken

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: thread safety of fhandler_base_overlapped
  2021-12-20 23:02 thread safety of fhandler_base_overlapped David McFarland
  2021-12-20 23:48 ` Ken Brown
@ 2021-12-20 23:58 ` Takashi Yano
  2021-12-21  0:03   ` Takashi Yano
  1 sibling, 1 reply; 5+ messages in thread
From: Takashi Yano @ 2021-12-20 23:58 UTC (permalink / raw)
  To: cygwin-developers

On Mon, 20 Dec 2021 19:02:56 -0400
David McFarland wrote:
> I'm seeing a problem where occasionally write() returns EAGAIN on a
> blocking pipe. I believe this happens when multiple threads are writing
> concurrently to the pipe, and the overlapped structure is modified by
> one thread while another one is between calls to WFMO and
> GetOverlappedResult.
> 
> I found this change:
> 
> =====
> commit 2a4b1de773e6159ea8197b6fc3f7e4845479b476
> Author: Corinna Vinschen <corinna@vinschen.de>
> Date:   Mon Mar 23 21:06:00 2020 +0100
> 
>     Cygwin: serial: use per call OVERLAPPED structs
> 
>     Sharing the OVERLAPPED struct and event object in there between
>     read and select calls in the fhandler might have been a nice
>     optimization way back when, but it is a dangerous, not thread-safe
>     approach.  Fix this by creating per-fhandler, per-call OVERLAPPED
>     structs and event objects on demand.
> 
>     Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
> =====
> 
> which seems to fix a similar problem in fhandler_serial.
> 
> Is is possible that this needs to be done for fhandler_base_overlapped
> as well? I'll try using per-call buffers to see if it fixes my problem.

Pipe code was completely overhauled. Could you please try
the latest developer snapshot?
https://cygwin.com/snapshots/

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: thread safety of fhandler_base_overlapped
  2021-12-20 23:58 ` Takashi Yano
@ 2021-12-21  0:03   ` Takashi Yano
  2021-12-21  0:59     ` David McFarland
  0 siblings, 1 reply; 5+ messages in thread
From: Takashi Yano @ 2021-12-21  0:03 UTC (permalink / raw)
  To: cygwin-developers

On Tue, 21 Dec 2021 08:58:13 +0900
Takashi Yano wrote:
> On Mon, 20 Dec 2021 19:02:56 -0400
> David McFarland wrote:
> > I'm seeing a problem where occasionally write() returns EAGAIN on a
> > blocking pipe. I believe this happens when multiple threads are writing
> > concurrently to the pipe, and the overlapped structure is modified by
> > one thread while another one is between calls to WFMO and
> > GetOverlappedResult.
> > 
> > I found this change:
> > 
> > =====
> > commit 2a4b1de773e6159ea8197b6fc3f7e4845479b476
> > Author: Corinna Vinschen <corinna@vinschen.de>
> > Date:   Mon Mar 23 21:06:00 2020 +0100
> > 
> >     Cygwin: serial: use per call OVERLAPPED structs
> > 
> >     Sharing the OVERLAPPED struct and event object in there between
> >     read and select calls in the fhandler might have been a nice
> >     optimization way back when, but it is a dangerous, not thread-safe
> >     approach.  Fix this by creating per-fhandler, per-call OVERLAPPED
> >     structs and event objects on demand.
> > 
> >     Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
> > =====
> > 
> > which seems to fix a similar problem in fhandler_serial.
> > 
> > Is is possible that this needs to be done for fhandler_base_overlapped
> > as well? I'll try using per-call buffers to see if it fixes my problem.
> 
> Pipe code was completely overhauled. Could you please try
> the latest developer snapshot?
> https://cygwin.com/snapshots/

Sorry, the new pipe code is also available in cygwin 3.3.3.
Do you have the problem in the cygwin 3.3.3?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: thread safety of fhandler_base_overlapped
  2021-12-21  0:03   ` Takashi Yano
@ 2021-12-21  0:59     ` David McFarland
  0 siblings, 0 replies; 5+ messages in thread
From: David McFarland @ 2021-12-21  0:59 UTC (permalink / raw)
  To: takashi.yano; +Cc: cygwin-developers

> Sorry, the new pipe code is also available in cygwin 3.3.3.
> Do you have the problem in the cygwin 3.3.3?

Sorry about that. I've been debugging in this environment on and off for
months, so I was pretty out of date. It does indeed appear to be fixed
on 3.3.3.

Thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-21  0:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-20 23:02 thread safety of fhandler_base_overlapped David McFarland
2021-12-20 23:48 ` Ken Brown
2021-12-20 23:58 ` Takashi Yano
2021-12-21  0:03   ` Takashi Yano
2021-12-21  0:59     ` David McFarland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).