Close ReactiveSocket on DuplexConnection close.

[NiteshKant]

Problem

Today, ReactiveSocket does not know when the underlying DuplexConnection is closed. This has two problems:

• The underlying connection close is only discovered on writing a KeepAlive frame or writing/reading other frames. Although this "works" but is not ideal as that means we have to wait till the KeepAlive duration or next write to know about something that is easily available from the transport layer.
• There is currently no way of notifying a user when a ReactiveSocket closes. onShutdown() only handles explicit shutdown() calls.

Proposal

We can add two methods to ReactiveSocket, viz.,

    /**
     * Close this {@code ReactiveSocket} upon subscribing to the returned {@code Publisher}
     *
     * <em>This method is idempotent and hence can be called as many times at any point with same outcome.</em>
     *
     * @return A {@code Publisher} that completes when this {@code ReactiveSocket} close is complete.
     */
    Publisher<Void> close();

and

    /**
     * Returns a {@code Publisher} that completes when this {@code ReactiveSocket} is closed. A {@code ReactiveSocket}
     * can be closed by explicitly calling {@link #close()} or when the underlying transport connection is closed.
     *
     * @return A {@code Publisher} that completes when this {@code ReactiveSocket} close is complete.
     */
    Publisher<Void> closeNotifier();

The above can replace the following methods that currently exist:

void onShutdown(Completable c);

void shutdown();

void close() throws Exception;

Same methods will be added to DuplexConnection thus wiring close() together and providing correct notifications to users when the ReactiveSocket is closed.

We can modify onShutdown() and shutdown() methods to the signature of my proposal but I think close() is more inline with other implementations of connections found elsewhere.

Current close() method is inherited from AutoCloseable which signifies synchronous execution as it return void.

[yschimke]

It seems a bit of a shame to lose AutoCloseable. it simplifies client usage, where often the synchronous close is useful with try-with-resources blocks.

[NiteshKant]

@yschimke I would like to hear more of the cases where try-with-resources is useful for real world applications. I would always end up doing Observable.concatWith(socket.close()) (in RxJava world ofcourse) as one can't really synchronously close the socket, it is always at the completion of an asynchronous call.

Having said that, I think we can always provide a utility method elsewhere which converts asynchronous close() to synchronous AutoClose implementation.

[robertroeser]

technically this wouldn't close until someone request(n) the close - so that could be a little weird.

why not use a CompletableFuture?

[yschimke]

@NiteshKant I agree but would probably differentiate between expert users, and normal users.

I think making the simplest most Java idiomatic thing work correctly (but not optimally) is a cost worth paying. e.g. the clarity of the sample code that will be cut and paste everywhere.

It's a lot easier for users to go from correct+simpler -> correct+optimal once they care enough.

[NiteshKant]

0.5.x has close() and onClose() methods on ReactiveSocket. Closing this issue.