ASPLes / nopoll

OpenSource WebSocket toolkit
http://www.aspl.es/nopoll
GNU Lesser General Public License v2.1
124 stars 74 forks source link

Return EWOULDBLOCK instead of retrying read & write #24

Closed jcelaya closed 7 years ago

jcelaya commented 7 years ago

This set of patches returns EWOULDBLOCK when a read or write operation expects more data, instead of retrying 50 times. This lets the application decide what to do in that idle time. In particular, this set covers two cases:

jcelaya commented 7 years ago

Any comment on this pull request?

francisbrosnan commented 7 years ago

Hello Javier, Thanks for reporting and the patch. I think it looks good (I don't remember why we did it like this). Because it changes expected functionality (this API before this change would have more chances to return data over slow links), just to keep in mind. I'm merging it, Best Regards,

francisbrosnan commented 7 years ago

Hello Javier,

I've just fixed a bug that was hidden behind this PR:

https://github.com/ASPLes/nopoll/commit/1cad0c12c7e861be86ff5ac32f2de497ceaf5750

I think it is relevant for your case.

In short, internal nopoll_conn_send_frame always reported bytes written as bytes requested. This wasn't a problem before due to retry mecanism removed/solved by this PR. That causes upper level to have a report about bytes written that is not the case (which is finally solved by next retries) but, if upper level keeps tracks about bytes written (because another protocol is running on top) that causes a bytes-mismatch.

This also have spotted a problem nopoll_conn_complete_pending_write by which the function reported total amount of bytes (including upper level requested bytes plus nopoll's bytes added due to headers). Now the function just report upper level bytes written (without including anything added by nopoll functioning).

Just let you know, Best Regards,

jcelaya commented 7 years ago

I see, thank you!

Javi

2017-05-11 18:47 GMT+02:00 Francis Brosnan Blázquez < notifications@github.com>:

Hello Javier,

I've just fixed a bug that was hidden behind this PR:

ASPLes/nopoll@1cad0c1 https://github.com/ASPLes/nopoll/commit/1cad0c12c7e861be86ff5ac32f2de497ceaf5750

I think it is relevant for your case.

In short, internal nopoll_conn_send_frame always reported bytes written as bytes requested. This wasn't a problem before due to retry mecanism removed/solved by this PR. That causes upper level to have a report about bytes written that is not the case (which is finally solved by next retries) but, if upper level keeps tracks about bytes written (because another protocol is running on top) that causes a bytes-mismatch.

This also have spotted a problem nopoll_conn_complete_pending_write by which the function reported total amount of bytes (including upper level requested bytes plus nopoll's bytes added due to headers). Now the function just report upper level bytes written (without including anything added by nopoll functioning).

Just let you know, Best Regards,

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ASPLes/nopoll/pull/24#issuecomment-300849141, or mute the thread https://github.com/notifications/unsubscribe-auth/AB9t_nnkMJFko1UnoH5BzJBKwFuHzsk5ks5r4zu8gaJpZM4NAONx .