<div><font>Thank you for your reply. <br></font></div><div><font>I will try it.<br></font></div><div><includetail><div> </div><div> </div><div style="font:Verdana normal 14px;color:#000;"><div style="FONT-SIZE: 12px;FONT-FAMILY: Arial Narrow;padding:2px 0 2px 0;">------------------ Original ------------------</div><div style="FONT-SIZE: 12px;background:#efefef;padding:8px;"><div id="menu_sender"><b>From: </b> "Andy Green"<andy@warmcat.com>;</div><div><b>Date: </b> Thu, Jun 4, 2020 02:38 PM</div><div><b>To: </b> "张立峰"<lifeng.zhang@melot.cn>; "libwebsockets"<libwebsockets@ml.libwebsockets.org>; <wbr></div><div></div><div><b>Subject: </b> Re: [Libwebsockets] We met an issue when using libwebsocket and fixed it, how can I submit the patch</div></div><div> </div><div style="position:relative;"><div id="tmpcontent_res"></div><br><br>On 6/4/20 7:16 AM, 张立峰 wrote:<br>> Hi, Andy:<br>> <br>> Recently, we met a crash on windows.<br>> Some thing like this:<br>> The program is a windows client using websocket protocol.<br>> 1. We create a thread as the service thread, calling lws_service. We <br>> intended to use this thread to service may ws client connections. -- <br>> Mark as thread A<br>> 2. And we call lws_client_connect_via_info from another thread to create <br>> the wsi struct.  -- Mark as thread B.<br><br>You can't do that.<br><br>> 3. After some normal case testing, every thing is fine. Then we disabled <br>> the local network (from windows`s device manager) to test network failed <br>> case.<br>> 4. It will crash<br><br>Yes, lws is not threadsafe, if you ignore that you will crash in many <br>exciting and varied ways.<br><br>> 5. After some code debug effort, we know that the wsi created in <br>> lws_client_connect_via_info, freed twice. One is before <br>> lws_client_connect_via_info return and in the thread B,  anther is in <br>> the lws_service and in the thread A (founding a closed socket, I think, <br>> in our case is in windows-service.c(_lws_plat_service_tsi)?)<br>> <br>> I tried to fix it like this:<br>> 1. In the /libwebsockets/lib/roles/http/client/client-handshake.c<br>> 2. move following code a little down, before a successfull wsi return:<br>> if (__insert_wsi_socket_into_fds(wsi->context, wsi))<br>>             goto try_next_result_closesock;<br>> <br>> I am not very confident for this code change. Please give some help, if <br>> this is right fix?<br><br>No... the right fix is to understand lws deliberately does not even try <br>to be threadsafe.  Its model is that everything that uses its apis - <br>with one exception - is on the same thread inside the event loop.  Your <br>current architecture goes against that and you won't get a "fix" trying <br>to use it that way.<br><br>You may only call lws apis from the "lws event loop thread", unless you <br>are calling the one threadsafe api, lws_cancel_service(), any use of lws <br>apis from another thread is strictly forbidden.<br><br>For use with other threads, you need to take the approach to have the <br>other threads ask the lws thread to do work that is related to lws.  For <br>example, define a mutex and struct in a shared memory area that all the <br>threads can access, lock it and add your request in there.  Then unlock <br>the shared area and call lws_cancel_service()<br><br>https://libwebsockets.org/git/libwebsockets/tree/minimal-examples/ws-server/minimal-ws-server-threads/protocol_lws_minimal.c#n124<br><br>to signal to the lws thread there is something to do.  Your protocol <br>will immediately receive a LWS_CALLBACK_EVENT_WAIT_CANCELLED callback <br>**on the lws thread context**<br><br>https://libwebsockets.org/git/libwebsockets/tree/minimal-examples/ws-server/minimal-ws-server-threads/protocol_lws_minimal.c#n266<br><br>in there it can lock the shared struct and find out what it is being <br>asked to do, and do it from the lws thread, unlock and go on.<br><br>You can, eg, put your prepared _connection_info struct in the shared <br>area, prepared by the other thread.  So although this is a deep change, <br>actually it's probably not going to cause a big code rewrite.  But this <br>change isn't optional.<br><br>-Andy<br><br>> Best wishes<br>> Terry Zhang<br>> 2020-6-4<br>> <br>> <br>> <br>> ------------------ Original ------------------<br>> *From: * "Andy Green"<andy@warmcat.com>;<br>> *Date: * Wed, Mar 4, 2020 09:03 PM<br>> *To: * "张立峰"<lifeng.zhang@melot.cn>; <br>> "libwebsockets"<libwebsockets@ml.libwebsockets.org>;<br>> *Subject: * Re: [Libwebsockets] We met an issue when using libwebsocket <br>> and fixed it, how can I submit the patch<br>> <br>> <br>> On 3/4/20 10:28 AM, 张立峰 wrote:<br>>  > Hi, everyone:<br>>  ><br>>  > When using libwebsockets as a client to communicate a ws server, we<br>>  > write out many messages in a queue when receive<br>>  > a LWS_CALLBACK_CLIENT_WRITEABLE. This may cause the ws server to close<br>>  > our connection.<br>>  ><br>>  > We found that when lws_write return value is smaller than requested, the<br>>  > connection will be close when we use lws_write next time.<br>>  ><br>>  > After check the source code of libwebsockets, we found the issue cause:<br>>  ><br>>  > The "wsi->ws->inside_frame" is not set to "0" after wsi->buflist_out has<br>>  > been sent. This will cause the wrong coded ws frame been sent.<br>> <br>> Thanks, I modified it a little bit for the cases ws is not enabled for<br>> build or it's http and not ws connection, and pushed it on master and<br>> v3.2-stable.<br>> <br>> -Andy<br></div></div><!--<![endif]--></includetail></div>