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 client NettyContext.dispose() does not change disposed state #345

Closed
mostroverkhov opened this issue May 3, 2018 · 7 comments
Closed
Labels
type/bug A general bug
Milestone

Comments

@mostroverkhov
Copy link
Contributor

Expected behavior

Client NettyContext.dispose() should eventually lead to

  • NettyContext.onClose() completion
  • NettyContext.isDisposed() = true

Actual behavior

  • NettyContext.onClose() is not completed
  • NettyContext.isDisposed() = false

Steps to reproduce

snippet

Reactor Netty version

0.7.6.RELEASE

JVM version (e.g. java -version)

java version "1.8.0_171"
Java(TM) SE Runtime Environment (build 1.8.0_171-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.171-b11, mixed mode)

OS version (e.g. uname -a)

Linux desktop 4.4.0-116-generic #140~14.04.1-Ubuntu SMP Fri Feb 16 09:25:20 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
@mostroverkhov
Copy link
Contributor Author

originally reported in rsocket/rsocket-java#492

@mostroverkhov
Copy link
Contributor Author

mostroverkhov commented May 3, 2018

@violetagg I have tested proposed change 51b33c3 on current master, and see NettyContext.isDisposed() true even without calling dispose() - expected It to be false before dispose and true after. Also onClose() still not triggered

@violetagg
Copy link
Member

@mostroverkhov this is because the server sends the CloseWebSocketFrame
you have this

httpServer = HttpServer.create(0)
                .newHandler((in, out) -> out.sendWebsocket((i, o) -> o.sendString(
                        Mono.just("test"))))
                .block(Duration.ofSeconds(30));

which means once the Mono completes the server will send CloseWebSocketFrame
and you can see in the logs the following

06:56:36.529 [reactor-http-nio-6] DEBUG r.ipc.netty.http.server.HttpServer - [id: 0xde242ff5, L:/127.0.0.1:59936 - R:/127.0.0.1:59937] WRITE: 6B
         +-------------------------------------------------+
         |  0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f |
+--------+-------------------------------------------------+----------------+
|00000000| 81 04 74 65 73 74                               |..test          |
+--------+-------------------------------------------------+----------------+
06:56:36.530 [reactor-http-nio-6] DEBUG r.ipc.netty.http.server.HttpServer - [id: 0xde242ff5, L:/127.0.0.1:59936 - R:/127.0.0.1:59937] FLUSH
06:56:36.531 [reactor-http-nio-6] DEBUG r.i.n.c.ChannelOperationsHandler - [id: 0xde242ff5, L:/127.0.0.1:59936 - R:/127.0.0.1:59937] Writing object CloseWebSocketFrame(data: UnpooledByteBufAllocator$InstrumentedUnpooledUnsafeHeapByteBuf(ridx: 0, widx: 0, cap: 0))

because of this you see that the context is disposed

if you change the test to

.newHandler((in, out) -> out.options(sendOptions -> sendOptions.flushOnEach())
                        .sendWebsocket((i, o) -> o.sendString(
                                Flux.interval(Duration.ofSeconds(1))
                                        .map(aLong -> aLong+""))))

then the server will not send CloseWebSocketFrame
and when you invoke dispose then the client will do that and will close the connection
for onClose you might be right I have to check that

@mostroverkhov
Copy link
Contributor Author

mostroverkhov commented May 3, 2018

So that's because my output stream completed to early - got it. Modified my example a bit and isDisposed() works as expected, thanks. Looking forward for onClose.

@violetagg
Copy link
Member

violetagg commented May 4, 2018

@mostroverkhov can you try the change from the PR #346 ? Thanks

@mostroverkhov
Copy link
Contributor Author

mostroverkhov commented May 4, 2018

@violetagg tested branch issue-345 - both reactor-netty and related rsocket-transport-netty. onClose() completes after dispose, so I think issue is resolved.

@violetagg
Copy link
Member

thanks for testing
fix committed

mostroverkhov added a commit to mostroverkhov/rsocket-kotlin that referenced this issue Jun 11, 2018
…al bug reactor/reactor-netty#345, which prevented RSocket Netty-Websocket transport from sending its close event via onClose()
robertroeser pushed a commit to rsocket/rsocket-kotlin that referenced this issue Jun 12, 2018
* * Possibility to reject setup by Server acceptor

* Client Keep-alive data support

* Client Keep-alive handler responds to Frames with `respond` flag set

* Introduce RSocketFactory ClientOptions & ServerOptions

* fix rare NoSuchElementException on Request-Response interaction

* update reactor-netty to 0.7.8.RELEASE, fixing Websocket client critical bug reactor/reactor-netty#345, which prevented RSocket Netty-Websocket transport from sending its close event via onClose()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants