Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LuaEvent is unaware of LuaSocket buffers #6

Open
Zash opened this issue Sep 9, 2014 · 8 comments
Open

LuaEvent is unaware of LuaSocket buffers #6

Zash opened this issue Sep 9, 2014 · 8 comments

Comments

@Zash
Copy link
Contributor

Zash commented Sep 9, 2014

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.

@harningt
Copy link
Owner

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.

@imaginator
Copy link

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

@harningt
Copy link
Owner

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

@harningt
Copy link
Owner

harningt commented Oct 2, 2015

lunarmodules/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.

@Zash
Copy link
Contributor Author

Zash commented Oct 3, 2015

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.

@harningt
Copy link
Owner

harningt commented Oct 5, 2015

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
Copy link
Owner

harningt commented Oct 5, 2015

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.

@Zash
Copy link
Contributor Author

Zash commented Jan 7, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants