runtests.pl: kill processes locking test log files by mback2k · Pull Request #6179 · curl/curl

mback2k · 2020-11-05T21:02:31Z

For now only required and implemented for Windows.

Ref: #6058

ghost · 2020-11-09T19:33:11Z

Congratulations 🎉. DeepCode analyzed your code in 0.315 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

mback2k · 2020-12-05T21:09:48Z

I will continue working on this. I also plan to implement Linux/macOS support for this via lsof.

bagder · 2020-12-21T16:05:05Z

on Linux and macOS the files aren't "locked" when another process is reading/writing them so what would the reason and idea be for this on those platforms?

mback2k · 2020-12-21T16:17:16Z

on Linux and macOS the files aren't "locked" when another process is reading/writing them so what would the reason and idea be for this on those platforms?

That is correct, but it would still be good to know if the files are kept open while they should already be closed. So the detection part is probably a good idea, but maybe not the process killing part.

mback2k · 2021-02-27T20:26:34Z

@bagder you were right, since the deletion of "locked" files won't fail, the detection part wouldn't even run like on Windows.

mback2k · 2021-02-27T20:26:53Z

Rebased and ready for review.

jay

I think you should make this behavior an option, like runtests.pl -f forcibly kill lingering processes or something. This seems like a diagnostic for us to run in the CI and not for default use.

mback2k · 2021-02-28T06:33:02Z

I consider it a general part for CI runs that should always be enabled, but yes: a flag for outside of CI makes sense.

jay

The commit message should say that this is a new option

While Msys2 has a pwd binary which supports -L, Msys1 only has a built-in one with that feature. Part of curl#6179

Introduce a new runtests.pl command option: -rm For now only required and implemented for Windows. Ignore stunnel logs due to long running processes. Requires Sysinternals handle[64].exe to be on PATH. Reviewed-by: Jay Satiro Ref: curl#6058 Closes curl#6179

While Msys2 has a pwd binary which supports -L, Msys1 only has a shell built-in with that feature. Reviewed-by: Jay Satiro Part of #6179

In the cases observed throughout the last year, `handle64` run once per test run, but with no action (match or task kill). It did not help with flakiness and seems redundant. runtests launched it (if present) in Cygwin/MSYS jobs too, where it probably shouldn't have, because we have seen no flakiness there. In CI the tool was present and launched in MSYS2 jobs, but not in Cygwin. After this patch the "clearlocks" warning remain in the log. They are consistently appearing once in every MSVC CI log, early in the tests: ``` test 3207 SKIPPED: curl lacks OpenSSL support [...START-OF-TESTS...] test 0003...[HTTP POST with auth and contents but with content-length set to 0] --pd---e--- OK (3 out of 1596, remaining: 17:50, took 1.423s, duration: 00:02) test 0007...[HTTP with cookie parser and header recording] --pd--oe--- OK (7 out of 1596, remaining: 07:51, took 1.485s, duration: 00:02) test 0006...[HTTP with simple cookie send] --pd---e--- OK (6 out of 1596, remaining: 09:11, took 1.488s, duration: 00:02) test 0005...[HTTP over proxy] --pd---e--- OK (5 out of 1596, remaining: 11:03, took 1.491s, duration: 00:02) CUSTOMBUILD : error : 169: cleardir(log/8/lock) failed [D:\a\curl\curl\bld\tests\test-ci.vcxproj] test 0001...[HTTP GET] --pd---e--- OK (1 out of 1596, remaining: 55:34, took 1.466s, duration: 00:02) test 0004...[Replaced internal and added custom HTTP headers] ``` Ref: https://github.com/curl/curl/actions/runs/13546192228/job/37858323380?pr=16484#step:14:167 Ref: e53523f #14859 Ref: 311c31e #6179 Follow-up to 3a8920e #16600 Closes #16484

In the cases observed throughout the last year, `handle64` run once per test run, but with no action (match or task kill). It did not help with flakiness and seems redundant. runtests launched it (if present) in Cygwin/MSYS jobs too, where it probably shouldn't have, because we have seen no flakiness there. In CI the tool was present and launched in MSYS2 jobs, but not in Cygwin. After this patch the "clearlocks" warning remain in the log. They are consistently appearing once in every MSVC CI log, early in the tests: ``` test 3207 SKIPPED: curl lacks OpenSSL support [...START-OF-TESTS...] test 0003...[HTTP POST with auth and contents but with content-length set to 0] --pd---e--- OK (3 out of 1596, remaining: 17:50, took 1.423s, duration: 00:02) test 0007...[HTTP with cookie parser and header recording] --pd--oe--- OK (7 out of 1596, remaining: 07:51, took 1.485s, duration: 00:02) test 0006...[HTTP with simple cookie send] --pd---e--- OK (6 out of 1596, remaining: 09:11, took 1.488s, duration: 00:02) test 0005...[HTTP over proxy] --pd---e--- OK (5 out of 1596, remaining: 11:03, took 1.491s, duration: 00:02) CUSTOMBUILD : error : 169: cleardir(log/8/lock) failed [D:\a\curl\curl\bld\tests\test-ci.vcxproj] test 0001...[HTTP GET] --pd---e--- OK (1 out of 1596, remaining: 55:34, took 1.466s, duration: 00:02) test 0004...[Replaced internal and added custom HTTP headers] ``` Ref: https://github.com/curl/curl/actions/runs/13546192228/job/37858323380?pr=16484#step:14:167 Ref: e53523f curl#14859 Ref: 311c31e curl#6179 Follow-up to 3a8920e curl#16600 Closes curl#16484

mback2k added tests Windows Windows-specific labels Nov 5, 2020

mback2k self-assigned this Nov 5, 2020

mback2k force-pushed the runtests-checklocks branch from c569903 to b7742e1 Compare November 6, 2020 20:48

mback2k force-pushed the runtests-checklocks branch from 05486b6 to 9dd808f Compare November 11, 2020 20:15

mback2k force-pushed the runtests-checklocks branch from 9dd808f to df9e661 Compare November 11, 2020 20:20

mback2k mentioned this pull request Nov 12, 2020

Weird mixup of test data in test suite run #6058

Closed

mback2k force-pushed the runtests-checklocks branch from 36ffbec to f25af18 Compare November 13, 2020 19:22

mback2k force-pushed the runtests-checklocks branch from f25af18 to 3f443f2 Compare November 14, 2020 09:17

mback2k force-pushed the runtests-checklocks branch from 3f443f2 to 7d0b643 Compare November 14, 2020 20:36

mback2k force-pushed the runtests-checklocks branch from 7d0b643 to a9c21b9 Compare November 15, 2020 19:36

mback2k mentioned this pull request Nov 16, 2020

tests/server/tftpd.c: close upload file after transfer #6209

Closed

mback2k force-pushed the runtests-checklocks branch from a9c21b9 to ffcdffc Compare November 16, 2020 19:20

mback2k force-pushed the runtests-checklocks branch from ffcdffc to 8e8d6e5 Compare November 20, 2020 07:14

mback2k force-pushed the runtests-checklocks branch from 8e8d6e5 to 8918776 Compare December 26, 2020 19:22

mback2k force-pushed the runtests-checklocks branch from 8918776 to 52b9674 Compare February 27, 2021 20:10

mback2k marked this pull request as ready for review February 27, 2021 20:26

mback2k requested review from MarcelRaad, bagder and jay February 27, 2021 20:26

jay reviewed Feb 28, 2021

View reviewed changes

Comment thread tests/pathhelp.pm Outdated

jay reviewed Feb 28, 2021

View reviewed changes

Comment thread tests/runtests.pl Outdated

jay reviewed Feb 28, 2021

View reviewed changes

Comment thread tests/runtests.pl Outdated

jay reviewed Feb 28, 2021

View reviewed changes

mback2k force-pushed the runtests-checklocks branch from 52b9674 to ec6cff5 Compare February 28, 2021 11:07

jay approved these changes Feb 28, 2021

View reviewed changes

mback2k added 2 commits February 28, 2021 22:03

pathhelp.pm: fix use of pwd -L in Msys environment

02fa331

While Msys2 has a pwd binary which supports -L, Msys1 only has a built-in one with that feature. Part of curl#6179

mback2k force-pushed the runtests-checklocks branch from ec6cff5 to 22a346c Compare February 28, 2021 21:07

mback2k added a commit that referenced this pull request Mar 1, 2021

pathhelp.pm: fix use of pwd -L in Msys environment

b2b3c91

While Msys2 has a pwd binary which supports -L, Msys1 only has a shell built-in with that feature. Reviewed-by: Jay Satiro Part of #6179

mback2k closed this in 311c31e Mar 1, 2021

vszakats mentioned this pull request Feb 25, 2025

runtests: drop logic calling the handle tool (Windows) #16484

Closed

Navigation Menu

Uh oh!

Uh oh!

runtests.pl: kill processes locking test log files - #6179

runtests.pl: kill processes locking test log files#6179
mback2k wants to merge 2 commits into
curl:masterfrom
mback2k:runtests-checklocks

mback2k commented Nov 5, 2020

Uh oh!

ghost commented Nov 9, 2020 •

edited by ghost

Loading

Uh oh!

mback2k commented Dec 5, 2020

Uh oh!

bagder commented Dec 21, 2020

Uh oh!

mback2k commented Dec 21, 2020

Uh oh!

mback2k commented Feb 27, 2021

Uh oh!

mback2k commented Feb 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jay left a comment

Uh oh!

mback2k commented Feb 28, 2021

Uh oh!

jay left a comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

mback2k commented Nov 5, 2020

Uh oh!

ghost commented Nov 9, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

👉 View analysis in DeepCode’s Dashboard | Configure the bot

Uh oh!

mback2k commented Dec 5, 2020

Uh oh!

bagder commented Dec 21, 2020

Uh oh!

mback2k commented Dec 21, 2020

Uh oh!

mback2k commented Feb 27, 2021

Uh oh!

mback2k commented Feb 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jay left a comment

Choose a reason for hiding this comment

Uh oh!

mback2k commented Feb 28, 2021

Uh oh!

jay left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

ghost commented Nov 9, 2020 •

edited by ghost

Loading