Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: fix conflict between Cygwin/msys and Windows PIDs #5188

Closed
wants to merge 4 commits into from

Conversation

mback2k
Copy link
Member

@mback2k mback2k commented Apr 5, 2020

Add 65536 to Windows PIDs to allow Windows specific treatment in POSIX Perl based test suite.

See also:

Replaces #5178

This will hopefully fix the random issues encountered with Windows CI builds as described in #5034.

@mback2k mback2k added tests Windows Windows-specific labels Apr 5, 2020
@mback2k mback2k self-assigned this Apr 5, 2020
@mback2k
Copy link
Member Author

mback2k commented Apr 5, 2020

TODO: I guess secureserver.pl (stunnel) and sshserver.pl (openssh) will also need to be updated.

@mback2k mback2k force-pushed the fix-cygwin-msys-pid-conflicts branch from ab911df to dee3288 Compare April 5, 2020 17:19
@jay
Copy link
Member

jay commented Apr 5, 2020

Just FYI Windows PIDs are often over 65535 on Enterprise systems that churn a lot of processes.
Capture

@bagder
Copy link
Member

bagder commented Apr 5, 2020

According to some sources a Windows pid can be anything within 32bit...

@mback2k
Copy link
Member Author

mback2k commented Apr 5, 2020

Yes, Windows PIDs can be anything, but Cygwin/msys PIDs are generated below 65536 at the moment. So offsetting Windows PIDs by 65536 to have separate "namespaces" will work until they are generated below MAX_INT-65536. Actually I have seen exactly that stackoverflow question, too.

I was initially thinking about using negative values for native Windows PIDs, but that would require investigation of changes to some 500 LoCs in the Perl scripts.

@mback2k mback2k force-pushed the fix-cygwin-msys-pid-conflicts branch 3 times, most recently from 6a0672f to 38d018e Compare April 10, 2020 16:48
Add 65536 to Windows PIDs to allow Windows specific treatment.

See also:
- https://cygwin.com/git/?p=newlib-cygwin.git;a=commit; ↵
  h=b5e1003722cb14235c4f166be72c09acdffc62ea
- https://cygwin.com/git/?p=newlib-cygwin.git;a=commit; ↵
  h=448cf5aa4b429d5a9cebf92a0da4ab4b5b6d23fe

Replaces curl#5178
Since the Windows versions of both programs would write Windows
PIDs to their pidfiles which we cannot handle, we need to use
our known perl.exe Cygwin/msys PID together with exec() in order
to tie the spawned processes to the existance of our perl.exe

The perl.exe that is executing secureserver.pl and sshserver.pl
has a Cygwin/msys PID, because it is started inside Cygwin/msys.
@mback2k mback2k force-pushed the fix-cygwin-msys-pid-conflicts branch from 38d018e to 055b7b2 Compare April 10, 2020 20:55
@mback2k mback2k marked this pull request as ready for review April 10, 2020 20:56
@mback2k mback2k requested review from jay and MarcelRaad April 10, 2020 21:01
@mback2k mback2k closed this in 3f60a9e Apr 11, 2020
mback2k added a commit that referenced this pull request Apr 11, 2020
mback2k added a commit that referenced this pull request Apr 11, 2020
Since the Windows versions of both programs would write Windows
PIDs to their pidfiles which we cannot handle, we need to use
our known perl.exe Cygwin/msys PID together with exec() in order
to tie the spawned processes to the existance of our perl.exe

The perl.exe that is executing secureserver.pl and sshserver.pl
has a Cygwin/msys PID, because it is started inside Cygwin/msys.

Related to #5188
mback2k added a commit that referenced this pull request Apr 11, 2020
Avoid potential overflow of huge PIDs on Windows.

Related to #5188
Assisted-by: Marcel Raad
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

None yet

4 participants