multi: start the loop over when handles are removed #16588
Conversation
Since more than one handle can be removed in a single call to multi_runsingle(), we cannot easily continue on the next node when a node has been removed since that node migth ALSO have been removed. Reported-by: Philippe Antoine
| @@ -2619,6 +2620,12 @@ CURLMcode curl_multi_perform(CURLM *m, int *running_handles) | |||
| if(result) | |||
| returncode = result; | |||
| } | |||
| if(num_alive != multi->num_alive) | |||
There was a problem hiding this comment.
I fear this might lead to some unforeseen complications due to restart leading multi_runsingle being called multiple times for early nodes in the llist.
One way to handle this would be to mark the nodes to be released with a flag and then only unlink and free them after the loop is done. This way the n pointer could never point to deallocated memory. It would require some other changes too, specifically skipping "going away" nodes, though. It'd also add extra iteration of the list when purging the going away nodes.
Other option would be to have some multi_runsingle version that'd maintain "next" pointer. If the node currently being released would be the "next" pointer, the pointer would be updated to be the next of the released node. Passing this pointer around would add fair bit of code though.
There was a problem hiding this comment.
Since curl_multi_perform() can be called by an application at any time, and repeatedly, there really should not be any side-effect by this. We know what some applications do this as well, in for example PHP programs it has been common to not do any select/wait at all but just busy-call it and that generally works fine.
I agree that this should be done nicer to avoid the loop restart, but I would rather have us fix the immediate bug here in a simple way, then do the bigger work of doing this in a better way in a separate follow-up.
Since more than one handle can be removed in a single call to multi_runsingle(), we cannot easily continue on the next node when a node has been removed since that node migth ALSO have been removed.
Reported-by: Philippe Antoine