luaevent icon indicating copy to clipboard operation
luaevent copied to clipboard

LuaEvent is unaware of LuaSocket buffers

Open Zash opened this issue 10 years ago • 8 comments

LuaSocket has an intermediate buffer for incoming data. When one calls conn:read(n), n bytes of data will be returned from this buffer, which is refilled from the underlying socket in 8k blocks (BUF_SIZE in buffer.h).

My guess is that this is to reduce the number of syscalls

However, because LuaEvent isn't aware of this buffer, this has the unfortunate side-effect that if you read less than BUF_SIZE, there may still be data left and no new read event will fire until new data comes in from the network.

A workaround is to always read everything, eg by :read()-ing "*a" or BUF_SIZE bytes.

Zash avatar Sep 09 '14 14:09 Zash

Will need to investigate what access to LuaSocket internals I have...

A fix would be to always read from the socket until you get a partial buffer and then wait for the results... I think my code currently handles this case, at least for the copas-style wrapping.

harningt avatar Sep 16 '14 16:09 harningt

We ran into a similar problem with luaevent https://github.com/project-isizwe/wifi-chat/issues/3#issuecomment-95525656 (larger stanzas between xmpp-component and xmpp-client was hanging around on the server before being flushed out) .

imaginator avatar Apr 23 '15 13:04 imaginator

Will review luasocket sources and propose a patch to expose this data if necessary.

harningt avatar Sep 28 '15 16:09 harningt

diegonehab/luasocket#150 created - although technically I think that the existing method of reading while waiting for the timeout implemented in the copas-style wrapper would solve this issue.

A raw example in code of what calls are being made with comments would help accelerate closure of the real issue.

harningt avatar Oct 02 '15 03:10 harningt

LuaSocket does have a :dirty() method that returns true if there is data in the buffer. I suppose either LuaEvent or its users could check this and either re-call a callback or set a short timeout to check it again.

Zash avatar Oct 03 '15 15:10 Zash

Thanks for the info, it looks like the necessary pieces are present to do this efficiently. Looking at copas (which the utility wrapper strives to emulate), the method by which we determine if we need to wait for more data is the same:

  • Try to read data
    • On timeout, put the socket in the queue to be notified later
    • On success, return all the desired data that we could get before timeout

Now - if someone sets a timeout on the socket that is an extended period of time, they will see a wait hang - but data shouldn't be "lost" until sometime later.

Now - if there is an issue with writing - that is a different case, although I think it is handled because luasocket doesn't use the buffer system there.

harningt avatar Oct 05 '15 14:10 harningt

I suppose I am omitting some items that copas exposes (such as the 'dirty' function) so I'll at the very least expose the missing relevant functions.

harningt avatar Oct 05 '15 14:10 harningt

Hm, now that I read this again I'm not sure it's clear that we're using 0s timeouts, and we read less data than what is available. Then the remaining data in LuaSockets buffer is left there.

Maybe we (Prosody) should just set a short timeout if socket:dirty() after reading.

Zash avatar Jan 07 '16 22:01 Zash