Bugs item #3265485, was opened at 2011-03-31 23:15
Message generated for change (Comment added) made by qiq
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3265485&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: crash
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Miroslav Spousta (qiq)
Assigned to: Daniel Stenberg (bagder)
Summary: multi_socket() incorrect behavior

Initial Comment:
After pulling the latest code from the libcurl git repository (f1c6cd42f474df5922b8bf812dd23773fa650f22), my program started to crash while invoking curl_multi_socket_action(). git bisect revealed the suspected commit to be 3eac14b43c62717cc14733aba6827c0c3d38dc9a. Brief inspection of the commit reveals that it seems to introduce an incorrect source code optimization in multi_socket():

+ /* note that this can possibly be NULL at this point */
+ conn = data->set.one_easy->easy_conn;
+
       /* If the pipeline is enabled, take the handle which is in the head of
          the pipeline. If we should write into the socket, take the send_pipe
          head. If we should read from the socket, take the recv_pipe head. */
- if(data->set.one_easy->easy_conn) {
+ if(conn) {
         if ((ev_bitmask & CURL_POLL_OUT) &&
- data->set.one_easy->easy_conn->send_pipe &&
- data->set.one_easy->easy_conn->send_pipe->head)
- data = data->set.one_easy->easy_conn->send_pipe->head->ptr;
+ conn->send_pipe &&
+ conn->send_pipe->head)
+ data = conn->send_pipe->head->ptr;
         else if ((ev_bitmask & CURL_POLL_IN) &&
- data->set.one_easy->easy_conn->recv_pipe &&
- data->set.one_easy->easy_conn->recv_pipe->head)
- data = data->set.one_easy->easy_conn->recv_pipe->head->ptr;
+ conn->recv_pipe &&
+ conn->recv_pipe->head)
+ data = conn->recv_pipe->head->ptr;
       }
 
- if(data->set.one_easy->easy_conn) /* set socket event bitmask */
- data->set.one_easy->easy_conn->cselect_bits = ev_bitmask;
+ if(conn && !(conn->handler->protocol & PROT_LOCKEDBITS))
+ /* set socket event bitmask if they're not locked */
+ conn->cselect_bits = ev_bitmask;

       do
         result = multi_runsingle(multi, now, data->set.one_easy);
       while (CURLM_CALL_MULTI_PERFORM == result);
...

conn is initialized as data->set.one_easy->easy_conn, but it is not updated later on, when data is possibly changed. In addition, it seems that multi_runsingle() may change data->set.one_easy->easy_conn to NULL (e.g. in case of CURLM_STATE_DONE), which is not properly detected if conn is used instead of data->set.one_easy->easy_conn.

Solution would be to remove conn pointer and use data->set.one_easy->easy_conn instead.

OS: Ubuntu 10.04, i686

----------------------------------------------------------------------

Comment By: Miroslav Spousta (qiq)
Date: 2011-04-02 13:15

Message:
Unfortunately not, the problem with setting change
data->set.one_easy->easy_conn to NULL by multi_runsingle() remains. If I
set conn again just after multi_runsingle(), it works correctly.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2011-04-02 09:30

Message:
Thanks for your report. This shows our test suite lacks a good test case
for exactly this situation...

Have a look at my smaller fix that instead re-assign conn if data is
updated. Does it work for you?

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3265485&group_id=976
Received on 2011-04-02