-
Notifications
You must be signed in to change notification settings - Fork 267
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
Configurable write timeout for websocket server #1554
Conversation
This pull request introduces 1 alert and fixes 1 when merging 9b11699 into 5f5e610 - view on LGTM.com new alerts:
fixed alerts:
|
This pull request fixes 1 alert when merging 38d404b into c94ab4a - view on LGTM.com fixed alerts:
|
This pull request fixes 1 alert when merging dff86bf into 31b85cd - view on LGTM.com fixed alerts:
|
This pull request fixes 1 alert when merging d733009 into 31b85cd - view on LGTM.com fixed alerts:
|
rskj-core/src/main/java/co/rsk/rpc/netty/RskWebSocketJsonRpcHandler.java
Outdated
Show resolved
Hide resolved
rskj-core/src/main/java/co/rsk/rpc/netty/RskWebSocketJsonRpcHandler.java
Outdated
Show resolved
Hide resolved
rskj-core/src/main/java/co/rsk/rpc/netty/RskWebSocketJsonRpcHandler.java
Outdated
Show resolved
Hide resolved
rskj-core/src/main/java/co/rsk/rpc/netty/RskWebSocketServerProtocolHandler.java
Outdated
Show resolved
Hide resolved
rskj-core/src/main/java/co/rsk/rpc/netty/Web3WebSocketServer.java
Outdated
Show resolved
Hide resolved
rskj-core/src/test/java/co/rsk/rpc/netty/Web3WebSocketServerTest.java
Outdated
Show resolved
Hide resolved
rskj-core/src/main/java/co/rsk/rpc/netty/RskWebSocketJsonRpcHandler.java
Outdated
Show resolved
Hide resolved
rskj-core/src/main/java/co/rsk/rpc/netty/RskWebSocketJsonRpcHandler.java
Outdated
Show resolved
Hide resolved
rskj-core/src/main/java/co/rsk/rpc/netty/Web3WebSocketServer.java
Outdated
Show resolved
Hide resolved
rskj-core/src/main/java/org/ethereum/config/SystemProperties.java
Outdated
Show resolved
Hide resolved
This pull request fixes 1 alert when merging bdc3eb8 into 31b85cd - view on LGTM.com fixed alerts:
|
This pull request fixes 1 alert when merging 0b1ee3b into 31b85cd - view on LGTM.com fixed alerts:
|
This pull request fixes 1 alert when merging 4683d89 into 31b85cd - view on LGTM.com fixed alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm m8
This pull request fixes 1 alert when merging 61cc3a4 into 10a1941 - view on LGTM.com fixed alerts:
|
Description
Any websocket connection will be closed after a certain write timeout. Currently set to 30 seconds but it can be less (I think it should be less).
This implied
WriteTimeoutHandler
to theWeb3WebSocketServer
RskWebSocketJsonRpcHandler
to close a connection after exceding the timeout.RskJsonRpcHandler
toRskWebSocketJsonRpcHandler
, since it was only used for the websocket server.Also take account that I had to modify the
RskWebSocketJsonRpcHandler
, there where two reference counted objects that are producing memory leaks when the connection is closed due to exceding the timeout:This last one is also unnecesary, there's no need to retain a
msg
that will be passed anyway to the next handler.Unit Testing
This should be covered by an integration test, I've only added one assertion to make sure that the timeout has properly been set.
Also if you want to verify netty is not producing any other buffer leak, you can activate the leak decector by adding
Useful links