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

websocket transport disconnected occasionally #2081

Closed
tomnotcat opened this issue Apr 18, 2020 · 8 comments
Closed

websocket transport disconnected occasionally #2081

tomnotcat opened this issue Apr 18, 2020 · 8 comments
Labels

Comments

@tomnotcat
Copy link
Contributor

I use videoroom plugin with websocket transport, most time janus works well, but occasionally websocket will disconnect, the janus log file output:
[libwebsockets][ERR] ****** 0x7fe6cc0026e0: Sending new 187 (<81>~), pending truncated ...
It's illegal to do an lws_write outside of
the writable callback: fix your code

@lminiero
Copy link
Member

That's exactly what we do, if you even bothered to check the code. But it you think that's not the case, I will consider "fixing my code" when you say "please".

@tomnotcat
Copy link
Contributor Author

I'm sorry for your misunderstand, “fix your code” is part of libwebsockets log, Not added by me

@lminiero
Copy link
Member

Sorry about that, I shouldn't read mails early in the morning... 😊
It's weird we get that error, though, because we definitely only do writes from the lws callbacks. I'll reopen and check on Monday.

@tomnotcat
Copy link
Contributor Author

tomnotcat commented Apr 18, 2020

I'm appreciated for all your guys amazing work!

I googled and found this post, warmcat/libwebsockets#852

It seems we should test lws_send_pipe_choked before call lws_write when receive LWS_CALLBACK_SERVER_WRITEABLE

But I'm not sure whether this will fix the bug, I'm going to test it for a while.

@lminiero
Copy link
Member

Ah, thanks for the pointer! This comment makes sense indeed:

However two or more lws_write() inside the writable callback can also cause it, if the first one was unable to send everything.

So you should marshall what you want to write into a buffer and write it once (on lws_write() call) per writable callback.

as we do have a mechanism to send pending data. I'll have to check if we try to send more stuff in sequence, which is likely. I'll keep you posted.

@lminiero
Copy link
Member

Mh, no, that's not our case: I verified and in the LWS_CALLBACK_CLIENT_WRITEABLE callback we only do a singe lws_write and then call lws_callback_on_writable again before returning, waiting for the callback to be invoked again. This means we never call lws_write more than once in the same callback.

tomnotcat pushed a commit to tomnotcat/janus-gateway that referenced this issue Apr 26, 2020
@tomnotcat
Copy link
Contributor Author

I tested for some days in production environment,
After add lws_send_pipe_choked check, this error never happen again.
I created a pull request that fix this bug:
#2107

@lminiero
Copy link
Member

lminiero commented May 4, 2020

Closing as the fix was merged.

@lminiero lminiero closed this as completed May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants