[Libwebsockets] Truncated send handled by user not working as expected

Andy Green andy at warmcat.com
Mon Aug 18 13:45:05 CEST 2014



On 18 August 2014 19:22:13 GMT+08:00, nil100 at ig.com.br wrote:
> 
>
>Em 18/08/2014 08:00, Andy Green escreveu: 
>
>> On 18 August 2014 17:59:52 GMT+08:00, Roger Light <roger at atchoo.org>
>wrote:
>> On Mon, Aug 18, 2014 at 10:19 AM, Andy Green <andy at warmcat.com>
>wrote: On 18 August 2014 17:00:17 GMT+08:00, Roger Light
><roger at atchoo.org> wrote: I believe I could trigger the assert easily
>on Linux by calling libwebsocket_write() without checking
>lws_send_pipe_choked() first. I was checking that the socket was
>available to write, then attempting to write as much as possible until
>it blocked. So crudely, and iirc: case LWS_CALLBACK_SERVER_WRITEABLE:
>/* Gives assert */ while(have_data){ count = libwebsockets_write(len);
>if(count < 0) return 0; if(wlen != len) break; } Yeah you can't do
>that. Say you send a lot each time like 1MByte. The kernel won't accept
>that much for buffering, so a partial send is
> guaranteed. 
>
>> Once lws took on dealing with your partial send, flushing the
> buffered remainder 
>
>> has to take priority over everything else, since it has to get sent
> first before anything else. If I'm reading what Nilson said correctly,
>he has configured lws so that he is dealing with partial writes, not
>the
>library. The same is true in my case. In that situation I would expect
>to be able to call 
>
>What did you actually do to "configure lws so [you] are dealing with
>partial writes"?
>
>In my case I set no_buffer_all_partial_tx to 0 in the
>libwebsocket_protocols struct so that wsi->u.ws.clean_buffer is 1 (set
>at line 279) and so the condition at line 172 is satisfied. BTW, the
>header file says I should set it to 1 if I want to handle partials...
>what is the correct value? Is that the intended purpose of
>no_buffer_all_partial_tx?

Hum I see... no whatever its original idea, the code has changed around it.  It does something, but it seems to do the opposite of what the name suggests, not sure that's useful.  Left at 0, he will differentiate between clean and extension-rewritten situation and pass clean partials back to user code to deal with as you want.  The only problem there is about if he sent nothing just immediate -EAGAIN.  I think we have to do something to correct that but not sure what yet.

I think if you're sending HTTP, or maybe websocket where no extension will change state based on unsent and then resent websocket packets, you can imagine to do what you want.  At the moment the buffering is safe for all those cases.

But for all other cases - and it's common to have compression extension negotiated by a browser or other client - you just can't do what you're trying to do in the user code.

-Andy

>> libwebsockets_write() in a loop as above, and monitor the return code
>and possible errno so that I knew when the kernel buffers were full. If
>I replace libwebsockets_write() with just write() then the loop above
>is valid, I just need to handle the return code differently to detect
>EAGAIN and actual errors. 
>> 
>>> When there was a partial send, a writable callback is scheduled
>> automatically, and 
>> 
>>> before the user callback is notified, lws checks to see if there is
>a
>> pending partial send. 
>> 
>>> If so, it will send as much of the pending buffer as the kernel will
>> take. If it was 
>> 
>>> everything, then the partial send is marked as emptied out and the
>> next writeable 
>> 
>>> callback will get passed through to the user code again to start
>> sending something new. 
>> 
>>> What your asserting code does is ignore all that and just sit
>> looping, blocking and 
>> 
>>> spamming more new stuff into libwebsockets_write() every time
>without
>> going around 
>> 
>>> the service loop and let it dump what it buffered.
>> What the loop does is expect libwebsockets_write() to tell me if a
>write wasn't possible, just the way that write() and send() do. On the
>other hand --> case LWS_CALLBACK_SERVER_WRITEABLE: /* No assert */
>while(have_data && !lws_send_pipe_choked()){ count =
>libwebsockets_write(len); if(count < 0) return 0; if(wlen != len)
>break; } This makes sense because the test for if we can write means
>that the
> send() 
>
>> libwebsockets_write() eventually does never returns -EWOULDBLOCK or
> whatever, 
>
>> because we already checked he can actually send.
> I'd suggest this is the wrong approach. You're adding the requirement
>for an extra poll() call for each additional time 
>
>I justed talked about why the two chunks of code acted differently,
>from
>looking at that code, not created a "requirement".
>
>> libwebsockets_write() is called, versus having one extra call to
>libwebsockets_write() that returns to say the socket wasn't writeable.
>Put another way, if poll tells me that I can write, I can write until
>I'm told not to. I don't need to check poll again after each time I
>call write().
>
>Yes ignoring all that, the code is not reliable anyway, since the
>choked
>state of the pipe may change aynchronously due to whatever else in
>going
>on in the system. But it's still useful to loop and pack the pipe while
>it's relatively empty and you have many small writes.
>
>I'm happy if you think you identified a problem, and know how to fix
>it,
>but I'm still not sure what it is yet. (Maybe it's simpler to send a
>patch?)
>
>-Andy
>
>> Cheers, Roger
> 




More information about the Libwebsockets mailing list