Probably, an additional check for the "wait_data_conn" flag should be done in the ftp_multi_statemach function:

diff --git a/lib/ftp.c b/lib/ftp.c
index 7b7e113..15da5dc 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -3148,7 +3148,8 @@ static CURLcode ftp_multi_statemach(struct connectdata conn,
/ Check for the state outside of the Curl_socket_ready() return code checks
since at times we are in fact already in this state when this function
gets called. */

• *done = (ftpc->state == FTP_STOP) ? TRUE : FALSE;
• *done = (ftpc->state == FTP_STOP && ftpc->wait_data_conn != TRUE) ?
• TRUE : FALSE;

return result;
}

I'm not sure if this is the proper solution to this problem.

It makes your test case work at least I presume?

Test 591 seems very similar to your case but works fine for me. I suspect due to timing or similar.

ftp_do_more() really makes an effort already to not "complete" if ftpc->wait_data_conn is set TRUE. Any chance you can either single-step through that with a debugger or add some printf()s or something to help us understand which code path it takes it it returns with complete set TRUE when ftpc->wait_data_conn is also true?

I guess it's a timing issue. It's not easy to reproduce the problem.

Here comes more debug output with some explanations:

Connect data stream actively
FTP 0x75792bc0 (line 2132) state change from PORT to STOP
DO phase is complete2
STATE: DOING => DO_MORE handle 0x7573a008; line 1416 (connection #0)
When we are connected, DO MORE and then go DO_DO (multi_runsingle(), multi.c, line 1427)
FTP-DO-MORE called (ftp_do_more() ftp.c, line 3646)
FTP-DO-MORE: a transfer is about to take place (ftp_do_more(), ftp.c, line 3699)
FTP-DO-MORE: set_upload (ftp_do_more(), ftp.c, line 3721)
FTP 0x75792bc0 (line 3533) state change from STOP to STOR_TYPE
FTP 0x75792bc0 (line 1711) state change from STOR_TYPE to STOR
FTP 0x75792bc0 (line 2447) state change from STOR to STOP
Preparing for accepting server on data port
Checking for server connect
Data conn was not available immediately <===== here wait_data_conn is set to TRUE for the first time, but ftp_multi_statematch() doesn't check the flag and initializes the done flag to TRUE
We have control (multi_runsingle (), multi.c, line 1436)
STATE: DO_MORE => DO_DONE handle 0x7573a008; line 1444 (connection #0)
STATE: DO_DONE => WAITPERFORM handle 0x7573a008; line 1469 (connection #0)

STATE: WAITPERFORM => PERFORM handle 0x7573a008; line 1479 (connection #0)
STATE: PERFORM => DONE handle 0x7573a008; line 1637 (connection #0)
Curl_done
Remembering we are in dir ""
Connection #0 to host 192.168.0.100 left intact
transfer done ftp://192.168.0.100:21/test1_03300.jpg (No error-0)

Just summarizing:
"else if(data->set.upload)" block doesn't check for wait_data_conn and I think it's a problem, or maybe we shouldn't execute this part of code at all, in this case.

"else if(data->set.upload)" block doesn't check for wait_data_conn and I think it's a problem

Really? Because the if() just before this else checks for wait_data_conn and I figure that should be enough. Isn't it?

Also, you mentioned "ftp_multi_statematch() doesn't check the flag and initializes the done flag to TRUE", but isn't that driven from within ftp_do_more() ? If so, it checks the wait_data_conn flag after the multi_statemach function returns. What am I missing?

It's correct, that the if() just before the else checks for wait_data_conn, however at that moment this flag is not set to TRUE so the "else if" is executed and we enter ftp_multi_statemach() (line 3726). When the ftp_muliti_statemach returns, *completep is TRUE and wait_data_conn is TRUE.

Ah, ok. So would something like this fix it?

diff --git a/lib/ftp.c b/lib/ftp.c
index 7b7e113..2351638 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -3721,11 +3721,17 @@ static CURLcode ftp_do_more(struct connectdata *conn, int *completep)
       result = ftp_nb_type(conn, data->set.prefer_ascii, FTP_STOR_TYPE);
       if(result)
         return result;

       result = ftp_multi_statemach(conn, &complete);
-      *completep = (int)complete;
+      if(ftpc->wait_data_conn)
+        /* if we reach the end of the FTP state machine here, *complete will be
+           TRUE but so is ftpc->wait_data_conn, which says we need to wait for
+           the data connection and therefore we're not actually complete */
+        *completep = 0;
+      else
+        *completep = (int)complete;
     }
     else {
       /* download */
       ftp->downloadsize = -1; /* unknown as of yet */

Yes or the check of the wait_data_conn flag could be done in ftp_multi_statemach as suggested in my patch above.
Thanks!

Right, I just think my version here was slightly more conservative as I don't feel fully confident that putting it in the ftp_multi_statemach function doesn't have any drawbacks or side-effects.

Ok, I understand. Will you commit your fix? Thanks for your help.