Copilot commented on code in PR #221:
URL: https://github.com/apache/fluss-rust/pull/221#discussion_r2737354265
##########
crates/fluss/src/rpc/server_connection.rs:
##########
@@ -385,9 +385,15 @@ where
mut self: std::pin::Pin<&mut Self>,
cx: &mut std::task::Context<'_>,
) -> Poll<Self::Output> {
- match self.inner.as_mut().expect("no dropped").as_mut().poll(cx) {
+ let inner = self
+ .inner
+ .as_mut()
+ .expect("CancellationSafeFuture polled after completion");
+
+ match inner.as_mut().poll(cx) {
Poll::Ready(res) => {
self.done = true;
+ self.inner = None; // Prevent re-polling
Review Comment:
The PR title "chore: fix ci" and empty description don't adequately describe
the actual code changes. This PR fixes a potential bug in the
`CancellationSafeFuture` implementation by ensuring the inner future is cleared
after completion and improving the error message. Consider updating the PR
description to explain what CI failure this addresses and what the actual fix
is.
##########
crates/fluss/src/rpc/server_connection.rs:
##########
@@ -385,9 +385,15 @@ where
mut self: std::pin::Pin<&mut Self>,
cx: &mut std::task::Context<'_>,
) -> Poll<Self::Output> {
- match self.inner.as_mut().expect("no dropped").as_mut().poll(cx) {
+ let inner = self
+ .inner
+ .as_mut()
+ .expect("CancellationSafeFuture polled after completion");
+
+ match inner.as_mut().poll(cx) {
Poll::Ready(res) => {
self.done = true;
Review Comment:
The `done` field appears to be redundant after this change. Since
`self.inner` is now set to `None` upon completion (line 396), the condition
`self.inner.is_none()` is equivalent to `self.done == true`. Consider removing
the `done` field and checking `self.inner.is_none()` instead, or document why
both are needed.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
