FAQ Proposed Updates Summary and Preview Diff
Just want to get feedback on how these FAQ changes should be packaged as patches
(separate, series, single) and whether some of the changes should not be applied
at all.
Summary
General:
change setup references to use the Cygwin Setup program;
change Win32 references to Windows;
reword net release or distribution references;
emphasize 64-bit Cygwin and setup-x86_64 over 32-bit;
change see to place links around available wording;
change for or where appropriate;
change bash .{ext1,ext2} usage to .ext1/.ext2;
trim trailing spaces highlighted by git diff.
Files:
winsup/doc/faq-api.xml: add to timezone FAQ "Why isn't time zone set correctly?"
winsup/doc/faq-programming.xml
winsup/doc/faq-setup.xml: update setup-x86/_64 --help options for
"Does the Cygwin Setup program accept command-line arguments?";
update compressed setup file types;
remove install everything instructions and provide total size info, time
required, and 32-bit Cygwin address space limitations;
winsup/doc/faq-using.xml
winsup/doc/faq-what.xml
Preview diff attached.
--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in IEC units and prefixes, physical quantities in SI.]
diff --git a/winsup/doc/faq-api.xml b/winsup/doc/faq-api.xml
index 313f15d37c..8b149cd76f 100644
--- a/winsup/doc/faq-api.xml
+++ b/winsup/doc/faq-api.xml
@@ -5,7 +5,7 @@
Cygwin API Questions
-
+
How does everything work?
@@ -18,7 +18,7 @@ Windows into the C library. Then your apps should (ideally)
run on POSIX
systems (Unix/Linux) and Windows with no changes at the source level.
The C library is in a DLL, which makes basic applications quite small.
-And it allows relatively easy upgrades to the Win32/POSIX translation
+And it allows relatively easy upgrades to the Windows/POSIX translation
layer, providing that DLL changes stay backward-compatible.
For a good overview of Cygwin, you may want to read the Cygwin
@@ -140,7 +140,7 @@ spawn family of calls if possible.
Here's how it works:
Parent initializes a space in the Cygwin process table for child.
-Parent creates child suspended using Win32 CreateProcess call, giving
+Parent creates child suspended using Windows CreateProcess call, giving
the same path it was invoked with itself. Parent calls setjmp to save
its own context and then sets a pointer to this in the Cygwin shared
memory area (shared among all Cygwin tasks). Parent fills in the child's
@@ -326,7 +326,7 @@ name under the API.
E.g., the POSIX select system call can wait on a standard file handles
and handles to sockets. The select call in Winsock can only wait on
sockets. Because of this, the Cygwin dll does a lot of nasty stuff behind
-the scenes, trying to persuade various Winsock/Win32 functions to do what
+the scenes, trying to persuade various Winsock/Windows functions to do what
a Unix select would do.
If you are porting an application which already uses Winsock, then
@@ -337,11 +337,11 @@ direct calls to Winsock functions. If you use Cygwin,
use the POSIX API.
-I don't want Unix sockets, how do I use normal Win32
winsock?
+I don't want Unix sockets, how do I use normal Windows
winsock?
You don't. Look for the Mingw-w64 project to port applications using
-native Win32/Winsock functions. Cross compilers packages to build Mingw-w64
+native Windows/Winsock functions. Cross compilers packages to build Mingw-w64
targets are available in the Cygwin distro.
@@ -385,13 +385,34 @@ Cygwin version number details, check out the
-Why isn't timezone set correctly?
+Why isn't time zone set correctly?
-(Please note: This section has not yet been
updated for the latest net release.)
-
-Did you explicitly call tzset() before checking the value of timezone?
+Did you explicitly call tzset() before checking the value of time zone?
If not, you must do so.
+Time zone settings are updated by changes to the tzdata package included in all
+Cygwin installations.
+Have you run the Cygwin Setup program recently to update at least the
+tzdata
+package to include the latest current daylight saving (summer time) rules
+for dates of changes, hour offsets from UTC of time zones, and the
+geographic regions to which those rules and offsets apply.
+
+These changes are decided on by politicians, and announced
+by government officials, sometimes with short or no notice, so
+tzdata
+package updates are released at least a few, and sometimes several,
+times a year.
+As details of changes are not known until they are announced publicly by
+officials, often in foreign languages, and those details then have to be
+noticed, possibly translated, passed to, and picked up by the official
+tzdata
+source package maintainers, subsequently released in an update to the
+tzdata
+source package, and then those changes have to be picked up on and applied
+to the Cygwin
+tzdata
+package, which
RE: CPU microcode reported wrong in /proc/cpuinfo
> https://software.intel.com/security-software-guidance/insights/microcode-update-guidance Thanks, interesting reading. I think in my case the ucode update is done via the FIT method, though, as the latest microcode (0xA0E) is included in the re-packaged BIOS, and re-flashed into ROM along with it. -- Problem reports: https://cygwin.com/problems.html FAQ: https://cygwin.com/faq/ Documentation:https://cygwin.com/docs.html Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
RE: CPU microcode reported wrong in /proc/cpuinfo
> So I think you probably encountered another Windows sleep bug Quite possibly... The microcode version in the registry looks okay after wake-up from hibernate, though (but that subsumes the system POST and clean boot). > 0 - ucode loading supported by CPU - update available and successfully > applied - loaded ucode (Update...) != BIOS boot ucode (Previous Update...) Hmmm, cores1-3 "updated" after sleep? I think it must be a bug then, indeed. > If you are on Windows 10 you could report the issue to MS Nah, Windows 7 here... No longer "supported" -- Problem reports: https://cygwin.com/problems.html FAQ: https://cygwin.com/faq/ Documentation:https://cygwin.com/docs.html Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
Re: Cygwin package search and cygcheck -p urlencoding still broken on new web server
On 2020-07-16 07:42, Jon Turney wrote:
> On 16/07/2020 08:44, Marco Atzeri via Cygwin wrote:
>> On 16.07.2020 06:25, Brian Inglis wrote:
>>> On 2020-07-15 14:13, Marco Atzeri via Cygwin wrote:
> So it seems the urlencode is correct, but the urldecode isn't working.
>
> I've applied a one-character change which appears to fix things.
>> the git logs says that only Frank and Jon have recently worked on
>>
>> cgi-bin2/package-grep.cgi
>>
>> I would like to double check any possible modification before proposing a
>> patch to change the web server...
> This was actually fairly easy to reproduce locally, once I realized what was
> going on:
>
> $ export url="libssh%2Flibssh.h" ; printf '%b' "${url//%/\x}"
> libsshx2Flibssh.h
>
> $ export url="libssh%2Flibssh.h" ; printf '%b' "${ur//%/\\x}"
> libssh/libssh.h
Doh! Good catch! Been an annoyance for months.
I didn't realize cgi-bin scripts were visible under cygwin-htdocs/cgi-bin2.
--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in IEC units and prefixes, physical quantities in SI.]
--
Problem reports: https://cygwin.com/problems.html
FAQ: https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
Re: CPU microcode reported wrong in /proc/cpuinfo
On 2020-07-16 13:46, Brian Inglis wrote: > On 2020-07-16 08:56, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin wrote: > It may be possible that Windows executes a CPUID instruction, and then reads > MSR Intel states CPUID of leaf EAX = 1, so not reading that leaf may not update MSR. > 8BH IA32_BIOS_SIGN_ID to get the current value, without first preloading the > MSR > with zero, and rather than interpreting the zero result as unchanged, just > reports that value. -- Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada This email may be disturbing to some readers as it contains too much technical detail. Reader discretion is advised. [Data in IEC units and prefixes, physical quantities in SI.] -- Problem reports: https://cygwin.com/problems.html FAQ: https://cygwin.com/faq/ Documentation:https://cygwin.com/docs.html Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
Re: [PATCH 00/12] FIFO: fix multiple reader support
Hi Ken, On Jul 16 12:19, Ken Brown via Cygwin-patches wrote: > There were several flaws in my previous attempt to add support for > explicitly opening a FIFO multiple times for reading. (By > "explicitly" I mean by calling open rather than by calling > fork/exec/dup.) See > > https://sourceware.org/pipermail/cygwin/2020-July/245456.html > > for one indication of problems > > The most important flaw was that I tried to use an indirect, > unreliable method for determining whether there are writers open. > This is fixed in the second patch of this series by adding a member > '_nwriters' to struct fifo_shmem_t, which counts the number of open > writers. > > We now have to give writers access to the shared memory as well as > readers, so that they can increment _nwriters in open/fork/exec/dup > and decrement it in close. > > The other patches contain miscellaneous fixes/improvements. > > Ken Brown (12): > Cygwin: FIFO: fix problems finding new owner > Cygwin: FIFO: keep a writer count in shared memory > Cygwin: fhandler_fifo::hit_eof: improve reliability > Cygwin: FIFO: reduce I/O interleaving > Cygwin: FIFO: improve taking ownership in fifo_reader_thread > Cygwin: FIFO: fix indentation > Cygwin: FIFO: make certain errors non-fatal > Cygwin: FIFO: add missing lock > Cygwin: fhandler_fifo::take_ownership: don't set event unnecessarily > Cygwin: FIFO: allow take_ownership to be interrupted > Cygwin: FIFO: clean up > Cygwin: FIFO: update commentary > > winsup/cygwin/fhandler.h | 55 +-- > winsup/cygwin/fhandler_fifo.cc | 725 ++--- > winsup/cygwin/select.cc| 14 +- > 3 files changed, 433 insertions(+), 361 deletions(-) LGTM, please push. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer
Re: need to fix permissions
On Wed, Jul 15, 2020 at 7:46 PM wrote: > Thank you very much ! > > I didn't see anybody mention that attrib step, in all the _many_ docs that i read. You're welcome. This is probably because file attributes are not actually security attributes. (As an aside: Please remove the responder's email address when replying, as I have done above, to prevent potential spammer harvesting.) Bill -- Problem reports: https://cygwin.com/problems.html FAQ: https://cygwin.com/faq/ Documentation:https://cygwin.com/docs.html Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
Re: CPU microcode reported wrong in /proc/cpuinfo
On 2020-07-16 08:56, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin wrote: >> Managed to get this tested and applied thanks to your help and it has landed >> in >> new Cygwin 3.1.6 so please post your results and any further comments when >> you >> have a chance to upgrade and test. > > I checked it out in the new Cygwin 3.1.6, and it shows microcode version > correctly now, but assuming Windows was booted up from the initial power-on, > i.e. after BIOS POST. > > There's another problem, though... After wakeup (from sleep), Windows > reports microcode versions in the registry weirdly. > > Only the CPU0 (which in my case core 0) is reported with the same info that > was reported for all cores after the initial boot-up, all other cores are > reported with some old microcode version (I presume that's the microcode that > Windows has on its own in one of its CPU driver DLLs). > > So for core 0 it is as it was (and for all of them after the boot-up): > > C:\Windows\system32>regtool list -v > /proc/registry/HKEY_LOCAL_MACHINE/HARDWARE/DESCRIPTION/System/CentralProcessor/0/ > Component Information (REG_BINARY) = 00 00 00 00 00 00 00 00 > Identifier (REG_SZ) = "Intel64 Family 6 Model 23 Stepping 10" > Configuration Data (REG_FULL_RESOURCE_DESCRIPTOR) = ? > ProcessorNameString (REG_SZ) = "Intel(R) Xeon(R) CPU X5470 @ > 3.33GHz" > VendorIdentifier (REG_SZ) = "GenuineIntel" > FeatureSet (REG_DWORD) = 0x215b3ffe (559628286) > ~MHz (REG_DWORD) = 0x0d04 (3332) > Update Signature (REG_BINARY) = 00 00 00 00 0e 0a 00 00 > Update Status (REG_DWORD) = 0x0007 (7) > Previous Update Signature (REG_BINARY) = 00 00 00 00 0e 0a 00 00 > Platform ID (REG_DWORD) = 0x0040 (64) > > For other cores (1-3), it's this (after wake-up; but after the boot-up it > was the same as the above for core 0): > > C:\Windows\system32>regtool list -v > /proc/registry/HKEY_LOCAL_MACHINE/HARDWARE/DESCRIPTION/System/CentralProcessor/1/ > Component Information (REG_BINARY) = 00 00 00 00 00 00 00 00 > Identifier (REG_SZ) = "Intel64 Family 6 Model 23 Stepping 10" > Configuration Data (REG_FULL_RESOURCE_DESCRIPTOR) = ? > ProcessorNameString (REG_SZ) = "Intel(R) Xeon(R) CPU X5470 @ > 3.33GHz" > VendorIdentifier (REG_SZ) = "GenuineIntel" > FeatureSet (REG_DWORD) = 0x215b3ffe (559628286) > ~MHz (REG_DWORD) = 0x0d04 (3332) > Update Signature (REG_BINARY) = 00 00 00 00 0b 0a 00 00 > Update Status (REG_DWORD) = 0x (0) > Previous Update Signature (REG_BINARY) = 00 00 00 00 00 00 00 00 > Platform ID (REG_DWORD) = 0x0040 (64) > > Note that there's no "Previous Update Signature" in the latter case, and > "Update Status" is 0 (instead of 7, for core 0, and what it also was for > these same cores 1-3 after the boot-up). I'm being repetitive to underscore > the noted differences. > > So Cygwin reports these same values in its /proc/cpuinfo output (core0: > 0xA0E, cores1-3: 0xA0B)... Meanwhile, Intel CPU ID utility keeps saying the > CPU microcode version is still 0xA0E (they don't show "per-core" values, if > that was the thing at all), and so does HWiNFO64 (again for the CPU as a > whole). > > I'm not exactly sure how to read Window's registry values with cores on the > same CPU having different microcode versions (is that even possible?) Should not be unless the status differs or the hybrid bit is set saying CPUs are different. Depending on the CPU, ucode updates may be loaded on the boot core only, or on all cores: https://software.intel.com/security-software-guidance/insights/microcode-update-guidance It may be possible that Windows executes a CPUID instruction, and then reads MSR 8BH IA32_BIOS_SIGN_ID to get the current value, without first preloading the MSR with zero, and rather than interpreting the zero result as unchanged, just reports that value. So I think you probably encountered another Windows sleep bug, but VMs should not care if different values are returned after a sleep, if VMs are run at startup before any sleeps, although obviously starting a VM after a sleep could cause issues, especially on current Intel CPUs where mitigations required could be ucode version dependent if there are not feature or behaviour dependencies to check. If you are on Windows 10 you could report the issue to MS through the Feedback Hub app. > I tried to dig into what "Update Status" means, but I couldn't find any > useful information, unfortunately. > > I suspect that "0" means a successful update, but that would also mean that > Windows updated ucode in cores 1-3 from nothing to 0xA0B -- and I checked > that that's the latest microcode that is shipped with this version of > Windows, as a Windows Update, for this CPU. I found one post that said that > "Update Status" 6 would mean no matching update found, but there is no 6 in > my case. Correct AFAIK: 0 - ucode loading supported by CPU - update available and successfully applied - loaded ucode (Update...) != BIOS boot ucode (Previous Update...) 1
Re: name pipe problem: 1 writer, multiple concurrent readers
On 7/3/2020 7:09 AM, Ken Brown via Cygwin wrote: On 7/2/2020 1:50 PM, Morten Kjærulff via Cygwin wrote: I think we got a new release around the beginning of June, right? You said that there were still issues (I can confirm). If it can help, here is the output I see today of above scripts: $ ./tp.sh [...] 0 [fifo_reader] diff 1806 C:\cygwin\bin\diff.exe: *** fatal error - Can't update my handlers, Win32 error 87 Thanks for the report and the simple test case. Obviously I still have more work to do on this. Hi Morten, I've attempted to fix the bugs (see https://cygwin.com/pipermail/cygwin-patches/2020q3/010380.html). With these patches installed, I no longer see a fatal error or hanging diff processes. But your script still doesn't work as you expect. On a typical run of the parallel part, 6 or 7 of the 10 diff processes see the FIFO t.pip as empty. Here's a sample run under strace, so that I could see what was going on. 7 of the 10 diff processes saw t.pip as empty on this run. $ strace -o tpip.sh.strace sh -c ./tpip.sh PIDPPIDPGID WINPID TTY UIDSTIME COMMAND 130713061307 10932 pty1 197609 17:50:12 /usr/bin/bash 18426 1 18426 9360 cons0 197609 06:47:31 /usr/bin/sh 18429 18426 18426 13900 cons0 197609 06:47:32 /usr/bin/ps 1306 11306 3768 ? 197609 17:50:11 /usr/bin/mintty 184241307 18424 21840 pty1 197609 06:47:31 /usr/bin/strace result1 start 10 0 0 0 0 0 0 0 0 0 0 result1 end 0a1,2 > line1 > line2 0a1,2 > line1 > line2 0a1,2 > line1 > line2 0a1,2 > line1 > line2 0a1,2 > line1 > line2 0a1,2 0a1,2 > line1 > line1 > line2 > line2 result2 start 10 0 1 1 0 1 1 1 1 1 0 result2 end PIDPPIDPGID WINPID TTY UIDSTIME COMMAND 18480 18430 18426 15580 cons0 197609 06:47:33 /usr/bin/cp 130713061307 10932 pty1 197609 17:50:12 /usr/bin/bash 18484 18426 18426 21264 cons0 197609 06:47:44 /usr/bin/ps 18430 18426 18426 23472 cons0 197609 06:47:32 /usr/bin/sh 18426 1 18426 9360 cons0 197609 06:47:31 /usr/bin/sh 1306 11306 3768 ? 197609 17:50:11 /usr/bin/mintty 184241307 18424 21840 pty1 197609 06:47:31 /usr/bin/strace I'm attaching your script for ease of reference, and I'm attaching an excerpt from the strace output, to which I've added a few comments. The excerpt shows all open, close, read, and write system calls involving t.pip. Here's a summary of what you can see from those system calls in the parallel part of the script. In what follows, I've called the diff processes diff-1, diff-2,..., diff-10, and similarly for the cp processes (although there are only four of them). 1. cp-1 tries to open t.pip for writing and blocks. It unblocks when diff-1 opens t.pip for reading, and both processes run to completion as expected. 2. diff-2, diff-3, diff-4, and diff-5 try to open t.pip for reading, and they block until cp-2 opens it for writing. Then cp-2 writes 12 bytes to t.pip and closes it, and the four diff processes all try to read. diff-4 gets there first and reads the 12 bytes; it reads once more and sees EOF because there is no data available in the pipe and there are no writers open[1], so it considers those 12 bytes to constitute the file t.pip. It later exits with success. diff-2, diff-3, and diff-5 all complete their reads before cp-3 opens t.pip. They see EOF for the same reason as above, so t.pip appears empty and they exit with failure. 3. diff-6, diff-7, diff-8, diff-9, and diff-10 try to open t.pip for reading, and they block until cp-3 opens it for writing. Then cp-3 writes 12 bytes to t.pip and closes it, and the five diff processes all try to read. diff-10 gets there first and reads 12 bytes followed by EOF; it later exits with success. diff-6, diff-7, diff-8, and diff-9 all complete their reads before cp-4 opens t.pip. They see EOF, so t.pip appears empty and they exit with failure. 4. cp-4 tries to open t.pip and blocks because there are no more diff processes. I've run your script on Linux a few times, and it usually[2] behaves as you expect, with all diff processes succeeding. For reasons I don't understand, the diff and cp processes apparently alternate most of the time on Linux, rather than having 4 or 5 diff processes lumped together between the cp processes as on Cygwin. If someone can figure out the reason for the difference, and if it turns out to be related to the FIFO code, I could try to modify the code to make Cygwin behave more like Linux. Ken [1] From https://pubs.opengroup.org/onlinepubs/9699919799/functions/read.html: When attempting to read from an empty pipe or FIFO: * If no process has the pipe open for writing, read() shall return 0 to indicate end-of-file. [2] But I did
Re: Cygwin package search and cygcheck -p urlencoding still broken on new web server
On 16.07.2020 15:42, Jon Turney wrote:
On 16/07/2020 08:44, Marco Atzeri via Cygwin wrote:
On 16.07.2020 06:25, Brian Inglis wrote:
On 2020-07-15 14:13, Marco Atzeri via Cygwin wrote:
Problem: I want to search for
libssh/libssh.h
Using https://cygwin.com/packages/
If I put the name and press Go the URL is changed in
https://cygwin.com/cgi-bin2/package-grep.cgi?grep=libssh%2Flibssh.h=x86_64
and the output is
Found 0 matches for libsshx2Flibssh.h
^^^
Instead if I modify directly the URL in:
https://cygwin.com/cgi-bin2/package-grep.cgi?grep=libssh/libssh.h=x86_64
I think that this is an invalid URL, that just happens to work, so it's
correct that the link we produce is urlencoded.
(See e.g. https://en.wikipedia.org/wiki/Query_string#URL_encoding)
Found 3 matches for libssh/libssh.h
I guess that the current incapability is due to spurious urlencode in
cgi-bin2/package-grep.cgi
that is modifing the $param_grep
echo ''$file' -
'$desc''
Could someone double check my guess ?
So it seems the urlencode is correct, but the urldecode isn't working.
I've applied a one-character change which appears to fix things.
the git logs says that only Frank and Jon have recently worked on
cgi-bin2/package-grep.cgi
I would like to double check any possible modification before proposing a
patch to change the web server...
This was actually fairly easy to reproduce locally, once I realized what
was going on:
$ export url="libssh%2Flibssh.h" ; printf '%b' "${url//%/\x}"
libsshx2Flibssh.h
$ export url="libssh%2Flibssh.h" ; printf '%b' "${ur//%/\\x}"
libssh/libssh.h
Thanks Jon
now the search works fine also with cygcheck -p
--
Problem reports: https://cygwin.com/problems.html
FAQ: https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
Cygwin's sqlite3 modifies DLL search order
Hi all,
After some significant headache I discovered a problem introduced by
the Cygwin patches for sqlite3. The effect of this patch is that it
modifies the DLL search order for all subsequent DLL loads (by
filename instead of absolute path) in the application.
It contains the following patch in code which is executed in the
library's initialization:
sqlite3_vfs_register(, 1);
+#if !defined(SQLITE_OMIT_LOAD_EXTENSION)
+ if( cygwin_conv_path ){
+WCHAR buf[MAX_PATH];
+cygwin_conv_path(CCP_POSIX_TO_WIN_W, "/usr/bin",
+buf, MAX_PATH*sizeof(WCHAR));
+osSetDllDirectoryW(buf);
+#ifdef _WIN32
+ }else if( cygwin_conv_to_full_win32_path ){
+WCHAR buf[MAX_PATH];
+char *buf1 = (char *)buf;
+int i = MAX_PATH;
+cygwin_conv_to_full_win32_path("/usr/bin", buf1);
+while(--i>=0) buf[i] = buf1[i];
+osSetDllDirectoryW(buf);
+#endif
+ }
+#endif
+
The call to SetDllDirectoryW modifies the default DLL search path by
always inserting /usr/bin, superseding the normal $PATH search. Why
it does this I'm not sure. It seems related to this patch, though
it's taken on a rather different form over the years:
http://sqlite.1065341.n5.nabble.com/Wrong-filename-handling-in-sqlite3-load-extension-for-Cygwin-td74049.html
It is related to how sqlite3 searches for extension libraries. Though
I'd argue this patch is not even correct, since if the sqlite3 DLL is
installed somewhere other than /usr/bin (say /usr/local/bin) this
patch still won't be correct.
This is a problem since after using sqlite3 in an application,
subsequent DLL searches will always default to looking in /usr/bin
before any overrides I have on $PATH. Is there any way we can get
this fixed?
Thanks,
Madison
--
Problem reports: https://cygwin.com/problems.html
FAQ: https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
[PATCH 09/12] Cygwin: fhandler_fifo::take_ownership: don't set event unnecessarily
Don't set update_needed_evt if there's currently no owner. This will cause unnecessary churn once I'm the owner and am listening for connections. --- winsup/cygwin/fhandler_fifo.cc | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc index b6e172ddc..fd1695f40 100644 --- a/winsup/cygwin/fhandler_fifo.cc +++ b/winsup/cygwin/fhandler_fifo.cc @@ -1186,8 +1186,11 @@ fhandler_fifo::take_ownership () return; } set_pending_owner (me); + /* Wake up my fifo_reader_thread. */ owner_needed (); - SetEvent (update_needed_evt); + if (get_owner ()) +/* Wake up owner's fifo_reader_thread. */ +SetEvent (update_needed_evt); owner_unlock (); /* The reader threads should now do the transfer. */ WaitForSingleObject (owner_found_evt, INFINITE); -- 2.27.0
[PATCH 01/12] Cygwin: FIFO: fix problems finding new owner
When the owning reader closes and there are still readers open, the
owner needs to wait for a new owner to be found before closing its
fifo_client handlers. This involves a loop in which dec_nreaders is
called at the beginning and inc_nreaders is called at the end. Any
other reader that tries to access shmem->_nreaders during this loop
will therefore get an inaccurate answer.
Fix this by adding an nreaders method and using it instead of
dec_nreaders and inc_nreaders. Also add nreaders_lock to control
access to the shmem->_nreaders.
Make various other changes to improve the reliability of finding a new
owner.
---
winsup/cygwin/fhandler.h | 8 +++-
winsup/cygwin/fhandler_fifo.cc | 86 +-
2 files changed, 61 insertions(+), 33 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 7a28adc16..cf6daea06 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1328,7 +1328,7 @@ class fifo_shmem_t
{
LONG _nreaders;
fifo_reader_id_t _owner, _prev_owner, _pending_owner;
- af_unix_spinlock_t _owner_lock, _reading_lock, _reader_opening_lock;
+ af_unix_spinlock_t _owner_lock, _reading_lock, _reader_opening_lock,
_nreaders_lock;
/* Info about shared memory block used for temporary storage of the
owner's fc_handler list. */
@@ -1336,6 +1336,7 @@ class fifo_shmem_t
_sh_fc_handler_updated;
public:
+ int nreaders () const { return (int) _nreaders; }
int inc_nreaders () { return (int) InterlockedIncrement (&_nreaders); }
int dec_nreaders () { return (int) InterlockedDecrement (&_nreaders); }
@@ -1352,6 +1353,8 @@ public:
void reading_unlock () { _reading_lock.unlock (); }
void reader_opening_lock () { _reader_opening_lock.lock (); }
void reader_opening_unlock () { _reader_opening_lock.unlock (); }
+ void nreaders_lock () { _nreaders_lock.lock (); }
+ void nreaders_unlock () { _nreaders_lock.unlock (); }
int get_shared_nhandlers () const { return (int) _sh_nhandlers; }
void set_shared_nhandlers (int n) { InterlockedExchange (&_sh_nhandlers, n);
}
@@ -1420,8 +1423,11 @@ class fhandler_fifo: public fhandler_base
int reopen_shared_fc_handler ();
int remap_shared_fc_handler (size_t);
+ int nreaders () const { return shmem->nreaders (); }
int inc_nreaders () { return shmem->inc_nreaders (); }
int dec_nreaders () { return shmem->dec_nreaders (); }
+ void nreaders_lock () { shmem->nreaders_lock (); }
+ void nreaders_unlock () { shmem->nreaders_unlock (); }
fifo_reader_id_t get_prev_owner () const { return shmem->get_prev_owner (); }
void set_prev_owner (fifo_reader_id_t fr_id)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 3d34cdfab..2d4f7a97e 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -371,6 +371,8 @@ fhandler_fifo::record_connection (fifo_client_handler& fc,
int
fhandler_fifo::update_my_handlers ()
{
+ int ret = 0;
+
close_all_handlers ();
fifo_reader_id_t prev = get_prev_owner ();
if (!prev)
@@ -387,7 +389,7 @@ fhandler_fifo::update_my_handlers ()
{
debug_printf ("Can't open process of previous owner, %E");
__seterrno ();
- return -1;
+ goto out;
}
for (int i = 0; i < get_shared_nhandlers (); i++)
@@ -402,11 +404,13 @@ fhandler_fifo::update_my_handlers ()
debug_printf ("Can't duplicate handle of previous owner, %E");
--nhandlers;
__seterrno ();
- return -1;
+ goto out;
}
fc.state = shared_fc_handler[i].state;
}
- return 0;
+out:
+ set_prev_owner (null_fr_id);
+ return ret;
}
int
@@ -1414,41 +1418,59 @@ fhandler_fifo::close ()
{
if (reader)
{
- /* If we're the owner, try to find a new owner. */
- bool find_new_owner = false;
+ /* If we're the owner, we can't close our fc_handlers if a new
+owner might need to duplicate them. */
+ bool close_fc_ok = false;
cancel_reader_thread ();
- owner_lock ();
- if (get_owner () == me)
+ nreaders_lock ();
+ if (dec_nreaders () == 0)
{
- find_new_owner = true;
+ close_fc_ok = true;
+ ResetEvent (read_ready);
+ ResetEvent (owner_needed_evt);
+ ResetEvent (owner_found_evt);
set_owner (null_fr_id);
- set_prev_owner (me);
- owner_needed ();
+ set_prev_owner (null_fr_id);
+ set_pending_owner (null_fr_id);
+ set_shared_nhandlers (0);
}
- owner_unlock ();
- if (dec_nreaders () == 0)
- ResetEvent (read_ready);
- else if (find_new_owner && !IsEventSignalled (owner_found_evt))
+ else
{
- bool found = false;
- do
- if (dec_nreaders () >= 0)
- {
- /* There's still another reader open. */
- if (WaitForSingleObject (owner_found_evt, 1) == WAIT_OBJECT_0)
-
[PATCH 02/12] Cygwin: FIFO: keep a writer count in shared memory
When a reader opens, it needs to block if there are no writers open
(unless is is opened with O_NONBLOCK). This is easy for the first
reader to test, since it can just wait for a writer to signal that it
is open (via the write_ready event). But when a second reader wants
to open, all writers might have closed.
To check this, use a new '_nwriters' member of struct fifo_shmem_t,
which keeps track of the number of open writers. This should be more
reliable than the previous method.
Add nwriters_lock to control access to shmem->_nwriters, and remove
reader_opening_lock, which is no longer needed.
Previously only readers had access to the shared memory, but now
writers access it too so that they can increment _nwriters during
open/dup/fork/exec and decrement it during close.
Add an optional 'only_open' argument to create_shmem for use by
writers, which only open the shared memory rather than first trying to
create it. Since writers don't need to access the shared memory until
they have successfully connected to a pipe instance, they can safely
assume that a reader has already created the shared memory.
For debugging purposes, change create_shmem to return 1 instead of 0
when a reader successfully opens the shared memory after finding that
it had already been created.
Remove check_write_ready_evt, write_ready_ok_evt, and
check_write_ready(), which are no longer needed.
When opening a writer and looping to try to get a connection, recheck
read_ready at the top of the loop since the number of readers might
have changed.
To slightly speed up the process of opening the first reader, take
ownership immediately rather than waiting for the fifo_reader_thread
to handle it.
---
winsup/cygwin/fhandler.h | 27 ++--
winsup/cygwin/fhandler_fifo.cc | 263 ++---
2 files changed, 124 insertions(+), 166 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index cf6daea06..f034a110e 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1323,12 +1323,14 @@ struct fifo_reader_id_t
}
};
-/* Info needed by all readers of a FIFO, stored in named shared memory. */
+/* Info needed by all fhandlers for a given FIFO, stored in named
+ shared memory. This is mostly for readers, but writers need access
+ in order to update the count of open writers. */
class fifo_shmem_t
{
- LONG _nreaders;
+ LONG _nreaders, _nwriters;
fifo_reader_id_t _owner, _prev_owner, _pending_owner;
- af_unix_spinlock_t _owner_lock, _reading_lock, _reader_opening_lock,
_nreaders_lock;
+ af_unix_spinlock_t _owner_lock, _reading_lock, _nreaders_lock,
_nwriters_lock;
/* Info about shared memory block used for temporary storage of the
owner's fc_handler list. */
@@ -1339,6 +1341,9 @@ public:
int nreaders () const { return (int) _nreaders; }
int inc_nreaders () { return (int) InterlockedIncrement (&_nreaders); }
int dec_nreaders () { return (int) InterlockedDecrement (&_nreaders); }
+ int nwriters () const { return (int) _nwriters; }
+ int inc_nwriters () { return (int) InterlockedIncrement (&_nwriters); }
+ int dec_nwriters () { return (int) InterlockedDecrement (&_nwriters); }
fifo_reader_id_t get_owner () const { return _owner; }
void set_owner (fifo_reader_id_t fr_id) { _owner = fr_id; }
@@ -1351,10 +1356,10 @@ public:
void owner_unlock () { _owner_lock.unlock (); }
void reading_lock () { _reading_lock.lock (); }
void reading_unlock () { _reading_lock.unlock (); }
- void reader_opening_lock () { _reader_opening_lock.lock (); }
- void reader_opening_unlock () { _reader_opening_lock.unlock (); }
void nreaders_lock () { _nreaders_lock.lock (); }
void nreaders_unlock () { _nreaders_lock.unlock (); }
+ void nwriters_lock () { _nwriters_lock.lock (); }
+ void nwriters_unlock () { _nwriters_lock.unlock (); }
int get_shared_nhandlers () const { return (int) _sh_nhandlers; }
void set_shared_nhandlers (int n) { InterlockedExchange (&_sh_nhandlers, n);
}
@@ -1380,8 +1385,6 @@ class fhandler_fifo: public fhandler_base
HANDLE owner_needed_evt; /* The owner is closing. */
HANDLE owner_found_evt; /* A new owner has taken over. */
HANDLE update_needed_evt; /* shared_fc_handler needs updating. */
- HANDLE check_write_ready_evt; /* write_ready needs to be checked. */
- HANDLE write_ready_ok_evt;/* check_write_ready is done. */
/* Handles to non-shared events needed for fifo_reader_threads. */
HANDLE cancel_evt;/* Signal thread to terminate. */
@@ -1417,7 +1420,7 @@ class fhandler_fifo: public fhandler_base
void record_connection (fifo_client_handler&,
fifo_client_connect_state = fc_connected);
- int create_shmem ();
+ int create_shmem (bool only_open = false);
int reopen_shmem ();
int create_shared_fc_handler ();
int reopen_shared_fc_handler ();
@@ -1426,8 +1429,13 @@ class fhandler_fifo: public fhandler_base
int nreaders
[PATCH 04/12] Cygwin: FIFO: reduce I/O interleaving
Add a bool member 'last_read' to the fifo_client_handler structure,
which is set to true on a successful read. This is used by raw_read
as follows.
When raw_read is called, it first locates the writer (if any) for
which last_read is true. raw_read tries to read from that writer and
returns if there is input available. Otherwise, it proceeds to poll
all the writers, as before.
The effect of this is that if a writer writes some data that is only
partially read, the next attempt to read will continue to read from
the same writer. This should reduce the interleaving of output from
different writers.
---
winsup/cygwin/fhandler.h | 3 +-
winsup/cygwin/fhandler_fifo.cc | 55 +-
2 files changed, 50 insertions(+), 8 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index b5bfdd0b3..221c856e6 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1298,7 +1298,8 @@ struct fifo_client_handler
{
HANDLE h;
fifo_client_connect_state state;
- fifo_client_handler () : h (NULL), state (fc_unknown) {}
+ bool last_read; /* true if our last successful read was from this client. */
+ fifo_client_handler () : h (NULL), state (fc_unknown), last_read (false) {}
void close () { NtClose (h); }
fifo_client_connect_state set_state ();
};
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 3685cc0c2..afe21a468 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -404,6 +404,7 @@ fhandler_fifo::update_my_handlers ()
goto out;
}
fc.state = shared_fc_handler[i].state;
+ fc.last_read = shared_fc_handler[i].last_read;
}
out:
set_prev_owner (null_fr_id);
@@ -1200,15 +1201,56 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
/* No one else can take ownership while we hold the reading_lock. */
reading_lock ();
take_ownership ();
- /* Poll the connected clients for input. */
- int nconnected = 0;
+ /* Poll the connected clients for input. Make two passes. On
+the first pass, just try to read from the client from which
+we last read successfully. This should minimize
+interleaving of writes from different clients. */
fifo_client_lock ();
+ /* First pass. */
+ int j;
+ for (j = 0; j < nhandlers; j++)
+ if (fc_handler[j].last_read)
+ break;
+ if (j < nhandlers && fc_handler[j].state >= fc_closing)
+ {
+ NTSTATUS status;
+ IO_STATUS_BLOCK io;
+
+ status = NtReadFile (fc_handler[j].h, NULL, NULL, NULL,
+ , in_ptr, len, NULL, NULL);
+ switch (status)
+ {
+ case STATUS_SUCCESS:
+ case STATUS_BUFFER_OVERFLOW:
+ /* io.Information is supposedly valid in latter case. */
+ if (io.Information > 0)
+ {
+ len = io.Information;
+ fifo_client_unlock ();
+ reading_unlock ();
+ return;
+ }
+ break;
+ case STATUS_PIPE_EMPTY:
+ break;
+ case STATUS_PIPE_BROKEN:
+ fc_handler[j].state = fc_disconnected;
+ break;
+ default:
+ debug_printf ("NtReadFile status %y", status);
+ fc_handler[j].state = fc_error;
+ break;
+ }
+ fc_handler[j].last_read = false;
+ }
+
+ /* Second pass. */
+ int nconnected = 0;
for (int i = 0; i < nhandlers; i++)
if (fc_handler[i].state >= fc_closing)
{
NTSTATUS status;
IO_STATUS_BLOCK io;
- size_t nbytes = 0;
nconnected++;
status = NtReadFile (fc_handler[i].h, NULL, NULL, NULL,
@@ -1217,11 +1259,10 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
{
case STATUS_SUCCESS:
case STATUS_BUFFER_OVERFLOW:
- /* io.Information is supposedly valid. */
- nbytes = io.Information;
- if (nbytes > 0)
+ if (io.Information > 0)
{
- len = nbytes;
+ len = io.Information;
+ fc_handler[i].last_read = true;
fifo_client_unlock ();
reading_unlock ();
return;
--
2.27.0
[PATCH 07/12] Cygwin: FIFO: make certain errors non-fatal
If update_my_handlers fails to duplicate one or more handles, just
mark the corresponding handlers as being in an error state.
But if update_my_handlers is unable to open the process of the
previous owner, it's likely that something serious has gone wrong, so
we continue to make that a fatal error.
---
winsup/cygwin/fhandler_fifo.cc | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 69dda0811..91a276ee9 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -383,11 +383,7 @@ fhandler_fifo::update_my_handlers ()
else
prev_proc = OpenProcess (PROCESS_DUP_HANDLE, false, prev.winpid);
if (!prev_proc)
-{
- debug_printf ("Can't open process of previous owner, %E");
- __seterrno ();
- goto out;
-}
+api_fatal ("Can't open process of previous owner, %E");
for (int i = 0; i < get_shared_nhandlers (); i++)
{
@@ -399,14 +395,17 @@ fhandler_fifo::update_my_handlers ()
!close_on_exec (), DUPLICATE_SAME_ACCESS))
{
debug_printf ("Can't duplicate handle of previous owner, %E");
- --nhandlers;
__seterrno ();
- goto out;
+ fc.state = fc_error;
+ fc.last_read = false;
+ ret = -1;
+ }
+ else
+ {
+ fc.state = shared_fc_handler[i].state;
+ fc.last_read = shared_fc_handler[i].last_read;
}
- fc.state = shared_fc_handler[i].state;
- fc.last_read = shared_fc_handler[i].last_read;
}
-out:
set_prev_owner (null_fr_id);
return ret;
}
@@ -493,7 +492,7 @@ fhandler_fifo::fifo_reader_thread_func ()
set_owner (me);
set_pending_owner (null_fr_id);
if (update_my_handlers () < 0)
- api_fatal ("Can't update my handlers, %E");
+ debug_printf ("error updating my handlers, %E");
owner_found ();
owner_unlock ();
/* Fall through to owner_listen. */
--
2.27.0
[PATCH 12/12] Cygwin: FIFO: update commentary
--- winsup/cygwin/fhandler_fifo.cc | 21 + 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc index 30486304f..e9d0187d4 100644 --- a/winsup/cygwin/fhandler_fifo.cc +++ b/winsup/cygwin/fhandler_fifo.cc @@ -52,10 +52,23 @@ which is mostly idle. The thread wakes up if that reader might need to take ownership. - There is a block of shared memory, accessible to all readers, - that contains information needed for the owner change process. - It also contains some locks to prevent races and deadlocks - between the various threads. + There is a block of named shared memory, accessible to all + fhandlers for a given FIFO. It keeps track of the number of open + readers and writers; it contains information needed for the owner + change process; and it contains some locks to prevent races and + deadlocks between the various threads. + + The shared memory is created by the first reader to open the + FIFO. It is opened by subsequent readers and by all writers. It + is destroyed by Windows when the last handle to it is closed. + + If a handle to it somehow remains open after all processes + holding file descriptors to the FIFO have closed, the shared + memory can persist and be reused with stale data by the next + process that opens the FIFO. So far I've seen this happen only + as a result of a bug in the code, but there are some debug_printf + statements in fhandler_fifo::open to help detect this if it + happens again. At this writing, I know of only one application (Midnight Commander when running under tcsh) that *explicitly* opens two -- 2.27.0
[PATCH 05/12] Cygwin: FIFO: improve taking ownership in fifo_reader_thread
When a reader takes ownership in fifo_reader_thread, it now goes
directly to the part of the main loop that listens for a connection.
Previously it went back to the beginning of the loop.
Also, if the reader has to delay taking ownership because the previous
owner has not finished updating the shared fifo_client handlers, it
now checks to see if cancel_evt has been set. Previously it might
have had to spin its wheels unnecessarily only to eventually find that
its thread had been canceled.
---
winsup/cygwin/fhandler_fifo.cc | 44 ++
1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index afe21a468..1fb319fcf 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -462,12 +462,30 @@ fhandler_fifo::fifo_reader_thread_func ()
take_ownership = true;
else if (cur_owner != me)
idle = true;
- if (take_ownership)
+ else
+ /* I'm the owner. */
+ goto owner_listen;
+ if (idle)
+ {
+ owner_unlock ();
+ HANDLE w[2] = { owner_needed_evt, cancel_evt };
+ switch (WaitForMultipleObjects (2, w, false, INFINITE))
+ {
+ case WAIT_OBJECT_0:
+ continue;
+ case WAIT_OBJECT_0 + 1:
+ goto canceled;
+ default:
+ api_fatal ("WFMO failed, %E");
+ }
+ }
+ else if (take_ownership)
{
if (!shared_fc_handler_updated ())
{
owner_unlock ();
- yield ();
+ if (IsEventSignalled (cancel_evt))
+ goto canceled;
continue;
}
else
@@ -478,26 +496,11 @@ fhandler_fifo::fifo_reader_thread_func ()
api_fatal ("Can't update my handlers, %E");
owner_found ();
owner_unlock ();
- continue;
+ /* Fall through to owner_listen. */
}
}
- else if (idle)
- {
- owner_unlock ();
- HANDLE w[2] = { owner_needed_evt, cancel_evt };
- switch (WaitForMultipleObjects (2, w, false, INFINITE))
- {
- case WAIT_OBJECT_0:
- continue;
- case WAIT_OBJECT_0 + 1:
- goto canceled;
- default:
- api_fatal ("WFMO failed, %E");
- }
- }
- else
- {
- /* I'm the owner */
+
+owner_listen:
fifo_client_lock ();
cleanup_handlers ();
if (add_client_handler () < 0)
@@ -590,7 +593,6 @@ fhandler_fifo::fifo_reader_thread_func ()
fifo_client_unlock ();
if (cancel)
goto canceled;
- }
}
canceled:
if (conn_evt)
--
2.27.0
[PATCH 11/12] Cygwin: FIFO: clean up
Remove the fhandler_fifo::get_me method, which is no longer used.
Make the methods get_owner, set_owner, owner_lock, and owner_unlock
private.
---
winsup/cygwin/fhandler.h | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 0e0cfbd71..60bd27e00 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1437,6 +1437,8 @@ class fhandler_fifo: public fhandler_base
void nwriters_lock () { shmem->nwriters_lock (); }
void nwriters_unlock () { shmem->nwriters_unlock (); }
+ fifo_reader_id_t get_owner () const { return shmem->get_owner (); }
+ void set_owner (fifo_reader_id_t fr_id) { shmem->set_owner (fr_id); }
fifo_reader_id_t get_prev_owner () const { return shmem->get_prev_owner (); }
void set_prev_owner (fifo_reader_id_t fr_id)
{ shmem->set_prev_owner (fr_id); }
@@ -1444,6 +1446,8 @@ class fhandler_fifo: public fhandler_base
{ return shmem->get_pending_owner (); }
void set_pending_owner (fifo_reader_id_t fr_id)
{ shmem->set_pending_owner (fr_id); }
+ void owner_lock () { shmem->owner_lock (); }
+ void owner_unlock () { shmem->owner_unlock (); }
void owner_needed ()
{
@@ -1483,12 +1487,6 @@ public:
void fifo_client_lock () { _fifo_client_lock.lock (); }
void fifo_client_unlock () { _fifo_client_lock.unlock (); }
- fifo_reader_id_t get_me () const { return me; }
- fifo_reader_id_t get_owner () const { return shmem->get_owner (); }
- void set_owner (fifo_reader_id_t fr_id) { shmem->set_owner (fr_id); }
- void owner_lock () { shmem->owner_lock (); }
- void owner_unlock () { shmem->owner_unlock (); }
-
DWORD take_ownership ();
void reading_lock () { shmem->reading_lock (); }
void reading_unlock () { shmem->reading_unlock (); }
--
2.27.0
[PATCH 00/12] FIFO: fix multiple reader support
There were several flaws in my previous attempt to add support for explicitly opening a FIFO multiple times for reading. (By "explicitly" I mean by calling open rather than by calling fork/exec/dup.) See https://sourceware.org/pipermail/cygwin/2020-July/245456.html for one indication of problems The most important flaw was that I tried to use an indirect, unreliable method for determining whether there are writers open. This is fixed in the second patch of this series by adding a member '_nwriters' to struct fifo_shmem_t, which counts the number of open writers. We now have to give writers access to the shared memory as well as readers, so that they can increment _nwriters in open/fork/exec/dup and decrement it in close. The other patches contain miscellaneous fixes/improvements. Ken Brown (12): Cygwin: FIFO: fix problems finding new owner Cygwin: FIFO: keep a writer count in shared memory Cygwin: fhandler_fifo::hit_eof: improve reliability Cygwin: FIFO: reduce I/O interleaving Cygwin: FIFO: improve taking ownership in fifo_reader_thread Cygwin: FIFO: fix indentation Cygwin: FIFO: make certain errors non-fatal Cygwin: FIFO: add missing lock Cygwin: fhandler_fifo::take_ownership: don't set event unnecessarily Cygwin: FIFO: allow take_ownership to be interrupted Cygwin: FIFO: clean up Cygwin: FIFO: update commentary winsup/cygwin/fhandler.h | 55 +-- winsup/cygwin/fhandler_fifo.cc | 725 ++--- winsup/cygwin/select.cc| 14 +- 3 files changed, 433 insertions(+), 361 deletions(-) -- 2.27.0
[PATCH 08/12] Cygwin: FIFO: add missing lock
--- winsup/cygwin/fhandler_fifo.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc index 91a276ee9..b6e172ddc 100644 --- a/winsup/cygwin/fhandler_fifo.cc +++ b/winsup/cygwin/fhandler_fifo.cc @@ -422,7 +422,9 @@ fhandler_fifo::update_shared_handlers () set_shared_nhandlers (nhandlers); memcpy (shared_fc_handler, fc_handler, nhandlers * sizeof (fc_handler[0])); shared_fc_handler_updated (true); + owner_lock (); set_prev_owner (me); + owner_unlock (); return 0; } -- 2.27.0
[PATCH 03/12] Cygwin: fhandler_fifo::hit_eof: improve reliability
Use the writer count introduced in the previous commit to help detect
EOF. Drop the maybe_eof method, which is no longer needed.
---
winsup/cygwin/fhandler.h | 7 +++
winsup/cygwin/fhandler_fifo.cc | 26 ++
winsup/cygwin/select.cc| 3 +--
3 files changed, 6 insertions(+), 30 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index f034a110e..b5bfdd0b3 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1392,7 +1392,6 @@ class fhandler_fifo: public fhandler_base
UNICODE_STRING pipe_name;
WCHAR pipe_name_buf[CYGWIN_FIFO_PIPE_NAME_LEN + 1];
- bool _maybe_eof;
fifo_client_handler *fc_handler; /* Dynamically growing array. */
int shandlers; /* Size (capacity) of the array. */
int nhandlers; /* Number of elements in the array. */
@@ -1473,9 +1472,9 @@ class fhandler_fifo: public fhandler_base
public:
fhandler_fifo ();
- bool hit_eof ();
- bool maybe_eof () const { return _maybe_eof; }
- void maybe_eof (bool val) { _maybe_eof = val; }
+ /* Called if we appear to be at EOF after polling fc_handlers. */
+ bool hit_eof () const
+ { return !nwriters () && !IsEventSignalled (writer_opening); }
int get_nhandlers () const { return nhandlers; }
fifo_client_handler _fc_handler (int i) { return fc_handler[i]; }
PUNICODE_STRING get_pipe_name ();
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 26b24d019..3685cc0c2 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -87,7 +87,7 @@ fhandler_fifo::fhandler_fifo ():
fhandler_base (),
read_ready (NULL), write_ready (NULL), writer_opening (NULL),
owner_needed_evt (NULL), owner_found_evt (NULL), update_needed_evt (NULL),
- cancel_evt (NULL), thr_sync_evt (NULL), _maybe_eof (false),
+ cancel_evt (NULL), thr_sync_evt (NULL),
fc_handler (NULL), shandlers (0), nhandlers (0),
reader (false), writer (false), duplexer (false),
max_atomic_write (DEFAULT_PIPEBUFSIZE),
@@ -361,8 +361,6 @@ fhandler_fifo::record_connection (fifo_client_handler& fc,
fifo_client_connect_state s)
{
fc.state = s;
- maybe_eof (false);
- ResetEvent (writer_opening);
set_pipe_non_blocking (fc.h, true);
}
@@ -1173,25 +1171,6 @@ fhandler_fifo::raw_write (const void *ptr, size_t len)
return ret;
}
-/* A reader is at EOF if the pipe is empty and no writers are open.
- hit_eof is called by raw_read and select.cc:peek_fifo if it appears
- that we are at EOF after polling the fc_handlers. We recheck this
- in case a writer opened while we were polling. */
-bool
-fhandler_fifo::hit_eof ()
-{
- bool ret = maybe_eof () && !IsEventSignalled (writer_opening);
- if (ret)
-{
- yield ();
- /* Wait for the reader thread to finish recording any connection. */
- fifo_client_lock ();
- fifo_client_unlock ();
- ret = maybe_eof ();
-}
- return ret;
-}
-
/* Called from raw_read and select.cc:peek_fifo. */
void
fhandler_fifo::take_ownership ()
@@ -1261,9 +1240,8 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
break;
}
}
- maybe_eof (!nconnected && !IsEventSignalled (writer_opening));
fifo_client_unlock ();
- if (maybe_eof () && hit_eof ())
+ if (!nconnected && hit_eof ())
{
reading_unlock ();
len = 0;
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 9ae567c38..80d16f2a7 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -883,9 +883,8 @@ peek_fifo (select_record *s, bool from_select)
goto out;
}
}
- fh->maybe_eof (!nconnected);
fh->fifo_client_unlock ();
- if (fh->maybe_eof () && fh->hit_eof ())
+ if (!nconnected && fh->hit_eof ())
{
select_printf ("read: %s, saw EOF", fh->get_name ());
gotone += s->read_ready = true;
--
2.27.0
[PATCH 06/12] Cygwin: FIFO: fix indentation
---
winsup/cygwin/fhandler_fifo.cc | 168 -
1 file changed, 84 insertions(+), 84 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 1fb319fcf..69dda0811 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -501,98 +501,98 @@ fhandler_fifo::fifo_reader_thread_func ()
}
owner_listen:
- fifo_client_lock ();
- cleanup_handlers ();
- if (add_client_handler () < 0)
- api_fatal ("Can't add a client handler, %E");
-
- /* Listen for a writer to connect to the new client handler. */
- fifo_client_handler& fc = fc_handler[nhandlers - 1];
- fifo_client_unlock ();
- shared_fc_handler_updated (false);
- owner_unlock ();
- NTSTATUS status;
- IO_STATUS_BLOCK io;
- bool cancel = false;
- bool update = false;
+ fifo_client_lock ();
+ cleanup_handlers ();
+ if (add_client_handler () < 0)
+ api_fatal ("Can't add a client handler, %E");
- status = NtFsControlFile (fc.h, conn_evt, NULL, NULL, ,
- FSCTL_PIPE_LISTEN, NULL, 0, NULL, 0);
- if (status == STATUS_PENDING)
- {
- HANDLE w[3] = { conn_evt, update_needed_evt, cancel_evt };
- switch (WaitForMultipleObjects (3, w, false, INFINITE))
- {
- case WAIT_OBJECT_0:
- status = io.Status;
- debug_printf ("NtFsControlFile STATUS_PENDING, then %y",
- status);
- break;
- case WAIT_OBJECT_0 + 1:
- status = STATUS_WAIT_1;
- update = true;
- break;
- case WAIT_OBJECT_0 + 2:
- status = STATUS_THREAD_IS_TERMINATING;
- cancel = true;
- update = true;
- break;
- default:
- api_fatal ("WFMO failed, %E");
- }
- }
- else
- debug_printf ("NtFsControlFile status %y, no STATUS_PENDING",
- status);
- HANDLE ph = NULL;
- NTSTATUS status1;
+ /* Listen for a writer to connect to the new client handler. */
+ fifo_client_handler& fc = fc_handler[nhandlers - 1];
+ fifo_client_unlock ();
+ shared_fc_handler_updated (false);
+ owner_unlock ();
+ NTSTATUS status;
+ IO_STATUS_BLOCK io;
+ bool cancel = false;
+ bool update = false;
- fifo_client_lock ();
- switch (status)
+ status = NtFsControlFile (fc.h, conn_evt, NULL, NULL, ,
+ FSCTL_PIPE_LISTEN, NULL, 0, NULL, 0);
+ if (status == STATUS_PENDING)
+ {
+ HANDLE w[3] = { conn_evt, update_needed_evt, cancel_evt };
+ switch (WaitForMultipleObjects (3, w, false, INFINITE))
{
- case STATUS_SUCCESS:
- case STATUS_PIPE_CONNECTED:
- record_connection (fc);
+ case WAIT_OBJECT_0:
+ status = io.Status;
+ debug_printf ("NtFsControlFile STATUS_PENDING, then %y",
+ status);
break;
- case STATUS_PIPE_CLOSING:
- record_connection (fc, fc_closing);
+ case WAIT_OBJECT_0 + 1:
+ status = STATUS_WAIT_1;
+ update = true;
break;
- case STATUS_THREAD_IS_TERMINATING:
- case STATUS_WAIT_1:
- /* Try to connect a bogus client. Otherwise fc is still
-listening, and the next connection might not get recorded. */
- status1 = open_pipe (ph);
- WaitForSingleObject (conn_evt, INFINITE);
- if (NT_SUCCESS (status1))
- /* Bogus cilent connected. */
- delete_client_handler (nhandlers - 1);
- else
- /* Did a real client connect? */
- switch (io.Status)
- {
- case STATUS_SUCCESS:
- case STATUS_PIPE_CONNECTED:
- record_connection (fc);
- break;
- case STATUS_PIPE_CLOSING:
- record_connection (fc, fc_closing);
- break;
- default:
- debug_printf ("NtFsControlFile status %y after failing to
connect bogus client or real client", io.Status);
- fc.state = fc_unknown;
- break;
- }
+ case WAIT_OBJECT_0 + 2:
+ status = STATUS_THREAD_IS_TERMINATING;
+ cancel = true;
+ update = true;
break;
default:
- break;
+ api_fatal ("WFMO failed, %E");
}
- if (ph)
- NtClose (ph);
- if (update && update_shared_handlers () < 0)
-
[PATCH 10/12] Cygwin: FIFO: allow take_ownership to be interrupted
Use cygwait in take_ownership to allow interruption while waiting to
become owner. Return the cygwait return value or a suitable value to
indicate an error.
raw_read now checks the return value and acts accordingly.
---
winsup/cygwin/fhandler.h | 2 +-
winsup/cygwin/fhandler_fifo.cc | 54 ++
winsup/cygwin/select.cc| 11 ++-
3 files changed, 59 insertions(+), 8 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 221c856e6..0e0cfbd71 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1489,7 +1489,7 @@ public:
void owner_lock () { shmem->owner_lock (); }
void owner_unlock () { shmem->owner_unlock (); }
- void take_ownership ();
+ DWORD take_ownership ();
void reading_lock () { shmem->reading_lock (); }
void reading_unlock () { shmem->reading_unlock (); }
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index fd1695f40..30486304f 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -1175,15 +1175,16 @@ fhandler_fifo::raw_write (const void *ptr, size_t len)
return ret;
}
-/* Called from raw_read and select.cc:peek_fifo. */
-void
+/* Called from raw_read and select.cc:peek_fifo. Return WAIT_OBJECT_0
+ on success. */
+DWORD
fhandler_fifo::take_ownership ()
{
owner_lock ();
if (get_owner () == me)
{
owner_unlock ();
- return;
+ return WAIT_OBJECT_0;
}
set_pending_owner (me);
/* Wake up my fifo_reader_thread. */
@@ -1192,8 +1193,19 @@ fhandler_fifo::take_ownership ()
/* Wake up owner's fifo_reader_thread. */
SetEvent (update_needed_evt);
owner_unlock ();
- /* The reader threads should now do the transfer. */
- WaitForSingleObject (owner_found_evt, INFINITE);
+ /* The reader threads should now do the transfer. */
+ DWORD waitret = cygwait (owner_found_evt, cw_cancel | cw_sig_eintr);
+ owner_lock ();
+ if (waitret == WAIT_OBJECT_0
+ && (get_owner () != me || get_pending_owner ()))
+{
+ /* Something went wrong. Return WAIT_TIMEOUT, which can't be
+returned by the above cygwait call. */
+ set_pending_owner (null_fr_id);
+ waitret = WAIT_TIMEOUT;
+}
+ owner_unlock ();
+ return waitret;
}
void __reg3
@@ -1206,7 +1218,37 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
{
/* No one else can take ownership while we hold the reading_lock. */
reading_lock ();
- take_ownership ();
+ switch (take_ownership ())
+ {
+ case WAIT_OBJECT_0:
+ break;
+ case WAIT_SIGNALED:
+ if (_my_tls.call_signal_handler ())
+ {
+ reading_unlock ();
+ continue;
+ }
+ else
+ {
+ set_errno (EINTR);
+ reading_unlock ();
+ goto errout;
+ }
+ break;
+ case WAIT_CANCELED:
+ reading_unlock ();
+ pthread::static_cancel_self ();
+ break;
+ case WAIT_TIMEOUT:
+ reading_unlock ();
+ debug_printf ("take_ownership returned an unexpected result; retry");
+ continue;
+ default:
+ reading_unlock ();
+ debug_printf ("unknown error while trying to take ownership, %E");
+ goto errout;
+ }
+
/* Poll the connected clients for input. Make two passes. On
the first pass, just try to read from the client from which
we last read successfully. This should minimize
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 80d16f2a7..3f3f33fb5 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -867,7 +867,16 @@ peek_fifo (select_record *s, bool from_select)
}
fh->reading_lock ();
- fh->take_ownership ();
+ if (fh->take_ownership () != WAIT_OBJECT_0)
+ {
+ select_printf ("%s, unable to take ownership", fh->get_name ());
+ fh->reading_unlock ();
+ gotone += s->read_ready = true;
+ if (s->except_selected)
+ gotone += s->except_ready = true;
+ goto out;
+ }
+
fh->fifo_client_lock ();
int nconnected = 0;
for (int i = 0; i < fh->get_nhandlers (); i++)
--
2.27.0
RE: CPU microcode reported wrong in /proc/cpuinfo
> Managed to get this tested and applied thanks to your help and it has landed > in > new Cygwin 3.1.6 so please post your results and any further comments when you > have a chance to upgrade and test. I checked it out in the new Cygwin 3.1.6, and it shows microcode version correctly now, but assuming Windows was booted up from the initial power-on, i.e. after BIOS POST. There's another problem, though... After wakeup (from sleep), Windows reports microcode versions in the registry weirdly. Only the CPU0 (which in my case core 0) is reported with the same info that was reported for all cores after the initial boot-up, all other cores are reported with some old microcode version (I presume that's the microcode that Windows has on its own in one of its CPU driver DLLs). So for core 0 it is as it was (and for all of them after the boot-up): C:\Windows\system32>regtool list -v /proc/registry/HKEY_LOCAL_MACHINE/HARDWARE/DESCRIPTION/System/CentralProcessor/0/ Component Information (REG_BINARY) = 00 00 00 00 00 00 00 00 Identifier (REG_SZ) = "Intel64 Family 6 Model 23 Stepping 10" Configuration Data (REG_FULL_RESOURCE_DESCRIPTOR) = ? ProcessorNameString (REG_SZ) = "Intel(R) Xeon(R) CPU X5470 @ 3.33GHz" VendorIdentifier (REG_SZ) = "GenuineIntel" FeatureSet (REG_DWORD) = 0x215b3ffe (559628286) ~MHz (REG_DWORD) = 0x0d04 (3332) Update Signature (REG_BINARY) = 00 00 00 00 0e 0a 00 00 Update Status (REG_DWORD) = 0x0007 (7) Previous Update Signature (REG_BINARY) = 00 00 00 00 0e 0a 00 00 Platform ID (REG_DWORD) = 0x0040 (64) For other cores (1-3), it's this (after wake-up; but after the boot-up it was the same as the above for core 0): C:\Windows\system32>regtool list -v /proc/registry/HKEY_LOCAL_MACHINE/HARDWARE/DESCRIPTION/System/CentralProcessor/1/ Component Information (REG_BINARY) = 00 00 00 00 00 00 00 00 Identifier (REG_SZ) = "Intel64 Family 6 Model 23 Stepping 10" Configuration Data (REG_FULL_RESOURCE_DESCRIPTOR) = ? ProcessorNameString (REG_SZ) = "Intel(R) Xeon(R) CPU X5470 @ 3.33GHz" VendorIdentifier (REG_SZ) = "GenuineIntel" FeatureSet (REG_DWORD) = 0x215b3ffe (559628286) ~MHz (REG_DWORD) = 0x0d04 (3332) Update Signature (REG_BINARY) = 00 00 00 00 0b 0a 00 00 Update Status (REG_DWORD) = 0x (0) Previous Update Signature (REG_BINARY) = 00 00 00 00 00 00 00 00 Platform ID (REG_DWORD) = 0x0040 (64) Note that there's no "Previous Update Signature" in the latter case, and "Update Status" is 0 (instead of 7, for core 0, and what it also was for these same cores 1-3 after the boot-up). I'm being repetitive to underscore the noted differences. So Cygwin reports these same values in its /proc/cpuinfo output (core0: 0xA0E, cores1-3: 0xA0B)... Meanwhile, Intel CPU ID utility keeps saying the CPU microcode version is still 0xA0E (they don't show "per-core" values, if that was the thing at all), and so does HWiNFO64 (again for the CPU as a whole). I'm not exactly sure how to read Window's registry values with cores on the same CPU having different microcode versions (is that even possible?) I tried to dig into what "Update Status" means, but I couldn't find any useful information, unfortunately. I suspect that "0" means a successful update, but that would also mean that Windows updated ucode in cores 1-3 from nothing to 0xA0B -- and I checked that that's the latest microcode that is shipped with this version of Windows, as a Windows Update, for this CPU. I found one post that said that "Update Status" 6 would mean no matching update found, but there is no 6 in my case. An interesting fact is that when I run Linux in VMWare on Windows woken up from sleep, allocating two CPUs for the VM, I can get a mix of one 0xA0E and one 0xA0B ucode reported in "/proc/cpuinfo" for the cores, or it can be two 0xA0Bs. But it looks like Linux knows exactly it is run virtualized on VMWare, and so may not be reporting from the actual values from the MSRs for that. I also noticed that it does not attempt to load any microcode in this case. When I use VirtualBox for the same, I get a completely different microcode version output (but so far consistent and same for both cores), 0x60B (which I don't think is a valid value at all, TBH). Yet the same, it does not looks like it attempts to load any on its own because it knows it is run virtualized (yet it does not state exactly the host OS VM software unlike it does with VMWare). So, apologies for the long post. I just tried to be thorough ;-) Thanks, Anton -- Problem reports: https://cygwin.com/problems.html FAQ: https://cygwin.com/faq/ Documentation:https://cygwin.com/docs.html Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
Re: Cygwin package search and cygcheck -p urlencoding still broken on new web server
On 16/07/2020 08:44, Marco Atzeri via Cygwin wrote:
On 16.07.2020 06:25, Brian Inglis wrote:
On 2020-07-15 14:13, Marco Atzeri via Cygwin wrote:
Problem: I want to search for
libssh/libssh.h
Using https://cygwin.com/packages/
If I put the name and press Go the URL is changed in
https://cygwin.com/cgi-bin2/package-grep.cgi?grep=libssh%2Flibssh.h=x86_64
and the output is
Found 0 matches for libsshx2Flibssh.h
^^^
Instead if I modify directly the URL in:
https://cygwin.com/cgi-bin2/package-grep.cgi?grep=libssh/libssh.h=x86_64
I think that this is an invalid URL, that just happens to work, so it's
correct that the link we produce is urlencoded.
(See e.g. https://en.wikipedia.org/wiki/Query_string#URL_encoding)
Found 3 matches for libssh/libssh.h
I guess that the current incapability is due to spurious urlencode in
cgi-bin2/package-grep.cgi
that is modifing the $param_grep
echo ''$file' -
'$desc''
Could someone double check my guess ?
So it seems the urlencode is correct, but the urldecode isn't working.
I've applied a one-character change which appears to fix things.
the git logs says that only Frank and Jon have recently worked on
cgi-bin2/package-grep.cgi
I would like to double check any possible modification before proposing a
patch to change the web server...
This was actually fairly easy to reproduce locally, once I realized what
was going on:
$ export url="libssh%2Flibssh.h" ; printf '%b' "${url//%/\x}"
libsshx2Flibssh.h
$ export url="libssh%2Flibssh.h" ; printf '%b' "${ur//%/\\x}"
libssh/libssh.h
--
Problem reports: https://cygwin.com/problems.html
FAQ: https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
Re: [PATCH] Fix poll/select signal socket as write ready on connect failure
Hi Marc, On Jul 15 20:54, Marc Hoersken via Cygwin wrote: > Hello everyone, > > I identified an issue related to the way the events FD_CONNECT and FD_CLOSE > returned by WSAEnumNetworkEvents are currently handled in > winsup/cygwin/fhandler_socket_inet.cc. > > It seems like the code does not handle the fact that those events are > returned only once for a socket and if not acted upon by the calling program > may not be received again. This means poll and select are currently not > consistend about the socket still being writable after a connect failure. > The first call to poll or select would signal the socket as writable, but > not any following call. The first call consumes the FD_CONNECT and FD_CLOSE > events, regardless of the event mask supplied by the calling program. So > even if the calling program does not care about writability in the first > call, the events are consumed and following calls checking for writability > will not be able to detect a connection failure. > [...] > As far as I understand calling poll and/or select should not change/reset > the socket readyness state, therefore I created a simple fix which could be > used to solve this issue. Attached you will find a suggested patch to make > sure poll and select always signal writability of a connection failed > socket. With this patch applied the above example command failed with a > "Connection refused" as expected. > > This patch only fixes the behaviour regarding connection failure (during > FD_CONNECT), I am not sure if connection closure (during FD_CLOSE) is also > affected, but I was not able to find code handling the fact that FD_CLOSE is > only signalled once. > > Please take a look and thanks in advance! Thanks for the patch. I pushed it. But then I got second thoughts in terms of how to fix the issue. The reason is that the FD_CLOSE problem shouldn't exist, simply for the fact that we never remove FD_CLOSE from the events mask, see https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/fhandler_socket_inet.cc;hb=HEAD#l377 So, rather than setting FD_WRITE at some later point in the code, what about handling this where the other FD_CONNECT stuff is handled, by not erasing the FD_CONNECT bit, just like with FD_CLOSE? diff --git a/winsup/cygwin/fhandler_socket_inet.cc b/winsup/cygwin/fhandler_socket_inet.cc index e5b0d2d1443e..b64d96225db1 100644 --- a/winsup/cygwin/fhandler_socket_inet.cc +++ b/winsup/cygwin/fhandler_socket_inet.cc @@ -354,7 +354,12 @@ fhandler_socket_wsock::evaluate_events (const long event_mask, long , } else wsock_events->events |= FD_WRITE; - wsock_events->events &= ~FD_CONNECT; + /* Since FD_CONNECT is only given once, we have to keep FD_CONNECT +for connection failed sockets to have consistent behaviour in +programs calling poll/select multiple times. Example test to +non-listening port: curl -v 127.0.0.1:47 */ + if (connect_state () != connect_failed) + wsock_events->events &= ~FD_CONNECT; wsock_events->connect_errorcode = 0; } /* This test makes accept/connect behave as on Linux when accept/connect @@ -376,12 +381,6 @@ fhandler_socket_wsock::evaluate_events (const long event_mask, long , if (erase) wsock_events->events &= ~(events & ~(FD_WRITE | FD_CLOSE)); } - /* Since FD_CONNECT is only given once, we manually need to set - FD_WRITE for connection failed sockets to have consistent - behaviour in programs calling poll/select multiple times. - Example test to non-listening port: curl -v 127.0.0.1:47 */ - if ((connect_state () == connect_failed) && (event_mask & FD_WRITE)) -wsock_events->events |= FD_WRITE; UNLOCK_EVENTS; return ret; What do you think? Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer -- Problem reports: https://cygwin.com/problems.html FAQ: https://cygwin.com/faq/ Documentation:https://cygwin.com/docs.html Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
Re: [ITA] curl 7.71-1 for review
On 13.07.2020 13:44, Brian Inglis wrote: On 2020-07-12 12:33, Marco Atzeri via Cygwin-apps wrote: On 12.07.2020 19:26, Brian Inglis wrote: I would not mind picking up curl from Yaakov's list. Newest release 7.71.1 is available for review at: https://drive.google.com/open?id=1kXtPxXUcWwpCwjEdw2TABkiKRfEaccR 404. That’s an error. The requested URL was not found on this server. That’s all we know. Sorry, copy/paste missed final char: https://drive.google.com/open?id=1kXtPxXUcWwpCwjEdw2TABkiKRfEaccRk builds fine and the package looks ok. Testing is still running, but I changed the maintainership to you Regards Marco
Re: Cygwin package search and cygcheck -p urlencoding still broken on new web server
On 16.07.2020 06:25, Brian Inglis wrote: On 2020-07-15 14:13, Marco Atzeri via Cygwin wrote: Problem: I want to search for libssh/libssh.h Using https://cygwin.com/packages/ If I put the name and press Go the URL is changed in https://cygwin.com/cgi-bin2/package-grep.cgi?grep=libssh%2Flibssh.h=x86_64 and the output is Found 0 matches for libsshx2Flibssh.h ^^^ Instead if I modify directly the URL in: https://cygwin.com/cgi-bin2/package-grep.cgi?grep=libssh/libssh.h=x86_64 Found 3 matches for libssh/libssh.h I guess that the current incapability is due to spurious urlencode in cgi-bin2/package-grep.cgi that is modifing the $param_grep echo ''$file' - '$desc'' Could someone double check my guess ? No change since your report on May 2nd, and my cross-post to overseers: https://cygwin.com/pipermail/cygwin/2020-May/244684.html https://sourceware.org/pipermail/overseers/2020q2/017115.html where Frank suggested replacing badly urlencoded characters by ".": this works as the dot is not changed case $c in [a-zA-Z0-9.~_-]) printf "$c" ;; *) printf '%%%02X' "'$c" $ cygcheck -p libssh/libssh.h Found 0 matches for libsshx2flibssh.h $ cygcheck -p libssh.libssh.h Found 3 matches for libssh.libssh.h libssh-devel-0.7.3-1 - libssh-devel: SSH implementation library (installed binaries and support files) libssh-devel-0.7.5-1 - libssh-devel: SSH implementation library libssh-devel-0.8.7-1 - libssh-devel: SSH implementation library The previous break appears to have been fixed by or due to Achim: https://cygwin.com/pipermail/cygwin/2019-November/243112.html Ping Achim?! the git logs says that only Frank and Jon have recently worked on cgi-bin2/package-grep.cgi I would like to doble check any possible modification before proposing a patch to change the web server... Regards Marco -- Problem reports: https://cygwin.com/problems.html FAQ: https://cygwin.com/faq/ Documentation:https://cygwin.com/docs.html Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
