-
Notifications
You must be signed in to change notification settings - Fork 10
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
Fast client writes will fill up memory #4
Comments
By the way, I'm filing these issues partly as a way to probe whether this project might come back to life — it looks promising (compared to the other libevent-based WebSocket implementations I've looked at) but it's seen no activity for 2 years. I have fixes for all the issues I've filed, currently in a private fork, but I can submit them as PRs if there's interest. |
Thank you for pointing out the issues. This project was a hobby project of mine to learn libevent and the websockets protocol and API design in a library. I tried to design it in a way so that it is possible to replace Libevent with any other event library/OS api without having to change the external API. Which in some ways makes things more complicated than they need to be if all you are interested in is to use Libevent. Again this was just a challenge for myself. However as you have noticed there are a lot of issues still remaining. But I have kind of lost interest in the project myself. A big reason being that I don't really use the websocket protocol in any project. I have never used this library for real, other than my test clients (which explains all the issues you have found). When trying to get full SSL support working I kind of gave up. Because of how bad the OpenSSL init API is, I just did not manage to do it in a nice way which made me lose the "fun" part of it all. But apparently the new verisons have a new API that doesn't make it crash because you init it more than once in an application :) Here is a fork that has done some substantial changes that removes the "hiding" of Libevent and exposes things: However, if you have fixes for these issues I am glad to accept Pull Requests and merge them. If you are interested in becoming an active maintainer of the project I'm up for that as well. But I don't want to give you any false hopes that I might start maintaining it actively myself, since I have too many other projects going |
Hello from the author of the mentioned fork :) |
@alxvasilev hello, thanks for the reply. I have looked at some of your changes and I like many of your fixes. It's fun to see my hobby project actually has been useful for someone. But yea there are some parts of the fixes I would not merge right away. So you don't need to do a PR. But maybe I get some motivation to port parts of your changes back upstream some day :) Hopefully in the process making things acceptable for you to use "as is". Specifically I saw you removed the autobahn test suite. I guess since it requires additional dependencies? I find it good to have such tests to verify that any changes still adheres to the Websocket standard. |
The client code using libws has no way to tell how fast data is being written to the socket, so it can't tell how fast it can send messages. If it sends messages faster than the socket can write them, the libevent write buffers will start to pile up and increase memory usage. This also increases latency, since the client can end up creating messages that only get written to the socket much later.
(Think of a webcam that wants to send camera frames over the socket as JPEG images. If it starts sending messages at 60FPS, but the network bandwidth isn't enough to support that, then memory will start to fill up with pending frames, and the other side of the connection will see the video feed in slower than real-time. Instead, the webcam should wait until the websocket is ready for more data, and then write the current frame.)
This can be fixed by adding a new callback that tells the client that the socket has room to write. This is equivalent to the write callback provided by libevent; in fact libws's write callback can invoke the new callback.
The text was updated successfully, but these errors were encountered: