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

Jump to bottom

Test hangs in event mode #12033

Closed
1 of 3 tasks
dfandrich opened this issue Oct 4, 2023 · 5 comments
Closed
1 of 3 tasks

Test hangs in event mode #12033

dfandrich opened this issue Oct 4, 2023 · 5 comments

Comments

@dfandrich
Copy link
Contributor

dfandrich commented Oct 4, 2023 •
edited by bagder

I did this

Test 1233 (as well as 1272 and 1238, although I have less data about those) has been hanging 100% of the time in the event-based CI build ever since commit b5bb84c (PR #11939) . I can reproduce this 100% of the time on my local Linux machine by performing parallel tests enabling event mode and using the FTP keyword to select tests. The last log message produced by the curl under test is:

 0000: 229 Entering Passiv Mode (|||1|)

Running with the GOPHER keyword causes test 1272 to hang.

Running with the TFTP keyword cases test 1238 to hang for about 2000 seconds until it times out and fails.

The latter two test hangs can be reproduced easily without parallel mode, but the first one seems to need it.

  • fixed test 1233
  • fixed test 1238
  • fixed test 1272

I expected the following

The tests should pass without hanging.

curl/libcurl version

8.4.0-DEV

operating system

CI build machine:

Linux fv-az736-78 6.2.0-1012-azure #12~22.04.1-Ubuntu SMP Thu Sep 7 14:07:14 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
@dfandrich
Copy link
Contributor Author

@icing Maybe you have an idea abuot this?

@icing
Copy link
Contributor

icing commented Oct 5, 2023

Interesting. Well, first of all, thanks for tracking this down.

I will have a look if the introduced change can be done in a better way.

@dfandrich dfandrich mentioned this issue Oct 5, 2023
Open
@bagder
Copy link
Member

bagder commented Oct 5, 2023

Reverting b5bb84c makes it work again. But it is just painting over the problem, there should not need to be a timer expiring there.

@bagder bagder mentioned this issue Oct 5, 2023
Closed
@bagder
Copy link
Member

bagder commented Oct 6, 2023

Test 1233 is fixed by #12042.

bagder added a commit that referenced this issue Oct 6, 2023
@bagder
multi: set CURLM_CALL_MULTI_PERFORM after switch to DOING_MORE
6dd6654 
Since there is nothing to wait for there. Avoids the test 1233 hang
reported in #12033.

Reported-by: Dan Fandrich
Closes #12042
bagder added a commit that referenced this issue Oct 7, 2023
@bagder
multi: do CURLM_CALL_MULTI_PERFORM at two more places
e8509c1 
... when it does a state transition but there is no particular socket or
timer activity. This was made apparent when commit b5bb84c remove a
superflous timer expiry.

Reported-by: Dan Fandrich.
Fixes #12033
@bagder bagder mentioned this issue Oct 7, 2023
Closed
@bagder
Copy link
Member

bagder commented Oct 7, 2023

And the other two by #12056

@bagder bagder closed this as completed in 07008ee Oct 7, 2023
@ehuss ehuss mentioned this issue Oct 11, 2023
Merged
ehuss added a commit to ecnelises/curl-rust that referenced this issue Oct 11, 2023
@ehuss
Fix upload_lots for 8.4.0
6b7e62b 
This test is hanging on Linux with the 8.4.0 update.

My poor understanding is that `perform()` should not be used with
`curl_multi_socket_action` (that is, it should only use `perform` or
`action`, not both). Removing this seems to fix things.

Note that there were some other hang-related changes in the 8.4.0
release. I bisected the hang to curl/curl#11939,
which then uncovered some curl tests that were hanging in
curl/curl#12033. That then spawned off
two more changes to fix the hangs (both of which are also in 8.4.0).
However, this upload_lots was still failing. I think this is likely
due to an incorrect use of the API poorly interacting with the timer
changes in 8.4.0.
zuoxiaofeng pushed a commit to zuoxiaofeng/curl that referenced this issue Nov 28, 2023
@bagder @zuoxiaofeng
multi: set CURLM_CALL_MULTI_PERFORM after switch to DOING_MORE
ea010f0 
Since there is nothing to wait for there. Avoids the test 1233 hang
reported in curl#12033.

Reported-by: Dan Fandrich
Closes curl#12042
zuoxiaofeng pushed a commit to zuoxiaofeng/curl that referenced this issue Nov 28, 2023
@bagder @zuoxiaofeng
multi: do CURLM_CALL_MULTI_PERFORM at two more places
4f625c1 
... when it does a state transition but there is no particular socket or
timer activity. This was made apparent when commit b5bb84c removed a
superfluous timer expiry.

Reported-by: Dan Fandrich.
Fixes curl#12033
Closes curl#12056
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

multi: do CURLM_CALL_MULTI_PERFORM at two more places curl/curl
3 participants
@icing @bagder @dfandrich