From: Ken Brown <kbrown@cornell.edu>
To: cygwin-patches@cygwin.com
Subject: [PATCH 2/8] Cygwin: FIFO: fix timing issue with owner change
Date: Tue, 4 Aug 2020 08:55:01 -0400 [thread overview]
Message-ID: <20200804125507.8842-3-kbrown@cornell.edu> (raw)
In-Reply-To: <20200804125507.8842-1-kbrown@cornell.edu>
fhandler_fifo::take_ownership() tacitly assumes that the current
owner's fifo_reader_thread will be woken up from WFMO when
update_needed_evt is signaled. But it's possible that the the current
owner's fifo_reader_thread is at the beginning of its main loop rather
than in its WFMO call when that event is signaled.
In this case the owner will never see that the event has been
signaled, and it will never update the shared fifo_client_handlers.
The reader that wants to take ownership will then spin its wheels
forever.
Fix this by having the current owner call update_shared_handlers at
the beginning of its loop, if necessary.
---
winsup/cygwin/fhandler_fifo.cc | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index ee7f47c0c..7b87aab00 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -472,17 +472,34 @@ fhandler_fifo::fifo_reader_thread_func ()
if (pending_owner)
{
- if (pending_owner != me)
+ if (pending_owner == me)
+ take_ownership = true;
+ else if (cur_owner != me)
idle = true;
else
- take_ownership = true;
+ {
+ /* I'm the owner but someone else wants to be. Have I
+ already seen and reacted to update_needed_evt? */
+ if (WaitForSingleObject (update_needed_evt, 0) == WAIT_OBJECT_0)
+ {
+ /* No, I haven't. */
+ fifo_client_lock ();
+ if (update_shared_handlers () < 0)
+ api_fatal ("Can't update shared handlers, %E");
+ fifo_client_unlock ();
+ }
+ owner_unlock ();
+ /* Yield to pending owner. */
+ Sleep (1);
+ continue;
+ }
}
else if (!cur_owner)
take_ownership = true;
else if (cur_owner != me)
idle = true;
else
- /* I'm the owner. */
+ /* I'm the owner and there's no pending owner. */
goto owner_listen;
if (idle)
{
@@ -1212,7 +1229,7 @@ fhandler_fifo::take_ownership ()
/* Wake up my fifo_reader_thread. */
owner_needed ();
if (get_owner ())
- /* Wake up owner's fifo_reader_thread. */
+ /* Wake up the owner and request an update of the shared fc_handlers. */
SetEvent (update_needed_evt);
owner_unlock ();
/* The reader threads should now do the transfer. */
--
2.28.0
next prev parent reply other threads:[~2020-08-04 12:55 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-04 12:54 [PATCH 0/8] FIFO: bug fixes and small improvements Ken Brown
2020-08-04 12:55 ` [PATCH 1/8] Cygwin: FIFO: lock fixes Ken Brown
2020-08-04 12:55 ` Ken Brown [this message]
2020-08-04 12:55 ` [PATCH 3/8] Cygwin: FIFO: add a timeout to take_ownership Ken Brown
2020-08-04 12:55 ` [PATCH 4/8] Cygwin: FIFO: reorganize some fifo_client_handler methods Ken Brown
2020-08-04 12:55 ` [PATCH 5/8] Cygwin: FIFO: don't read from pipes that are closing Ken Brown
2020-08-04 12:55 ` [PATCH 6/8] Cygwin: FIFO: synchronize the fifo_reader and fifosel threads Ken Brown
2020-08-04 12:55 ` [PATCH 7/8] Cygwin: FIFO: fix indentation Ken Brown
2020-08-04 12:55 ` [PATCH 8/8] Cygwin: FIFO: add a third pass to raw_read Ken Brown
2020-08-04 15:04 ` [PATCH 0/8] FIFO: bug fixes and small improvements Corinna Vinschen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200804125507.8842-3-kbrown@cornell.edu \
--to=kbrown@cornell.edu \
--cc=cygwin-patches@cygwin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).