feat: Added AutoCloseable interface#397
Conversation
Added AutoCloseable interface to: - LifecycleInitializer - McpAsyncClient - McpAsyncServer - McpServerTransportProvider - McpSession - McpSyncServer - McpTransport - McpTransportSession for support try-with-resources block. Signed-off-by: YunKui Lu <luyunkui95@gmail.com>
134130
left a comment
There was a problem hiding this comment.
IMHO, current close() means immediately shutdown, and closeGracefully() is the graceful shutdown.
AutoClosable should be graceful shutdown as default, as ExecutorService does. see: https://stackoverflow.com/questions/41393417/why-does-the-executorservice-interface-not-implement-autocloseable
Learned something new again! I’m thinking — would it make sense to introduce a |
It would be great if the sdk supports close() with a graceful shutdown from the beginning, but currently not. If we change the close's behavior, it will be a breaking change in almost every library user. I think many users are familiar with using network libraries, which may require close resources, so the AutoClosable interface is not necessary. |
|
I'm not good at English. The "so the AutoClosable interface is not necessary" is just my opinion. I'm sorry if you felt bad. |
|
@134130 |
I've added the AutoCloseable interface to the following classes to support the try-with-resources statement:
Motivation and Context
In order to support the try-with-resources block for these classes.
How Has This Been Tested?
N/A
Breaking Changes
N/A
Types of changes
Checklist
Additional context