moq-lite: Close session on drop

[kixelated]

Summary

• Implement Drop for Session to automatically close the underlying transport when dropped
• Closes with error code 0 (Cancel) and reason "dropped"
• Fixes issue where old sessions remained active after reconnection, causing stale events (like announced broadcasts) to be replayed

Test plan

• just check passes
• Manual testing with client reconnection scenarios

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

A Drop implementation is added to the Session struct in rs/moq-lite/src/session.rs. The implementation calls the underlying session close method with Error::Cancel and the reason "dropped" when the Session is dropped. This ensures cleanup occurs automatically when Session goes out of scope. No changes are made to public method signatures; the existing explicit close method remains available alongside this destructor-based cleanup mechanism.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'moq-lite: Close session on drop' directly and clearly describes the main change: implementing automatic session closure on drop.
Description check	✅ Passed	The description explains the Drop implementation, the error handling strategy, and the bug it fixes, all directly related to the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@rs/moq-lite/src/session.rs`:
- Around line 46-49: The Drop impl for Session currently always calls
self.session.close(...), causing a double-close when Session::close(self, ...)
is called; modify the Session struct to hold an internal guard (e.g., a bool
like closed or an Option<SessionInner>/Option<Transport>) that tracks whether
the session has already been closed, update Session::close (the consuming
method) to set that guard before returning, and change impl Drop for Session to
check the guard and only call the internal close logic if the session is not
already closed; if needed, factor the actual close implementation into a private
fn close_inner(&mut self, code, reason) so both Session::close and Drop can
invoke it without consuming self.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Prevent double-close when Session::close() is called explicitly.

Because Session::close(self, …) consumes self, the new Drop runs immediately after and calls close again with Cancel/"dropped". That can override the caller’s error code/reason or cause a duplicate close on transports that aren’t idempotent. Consider guarding drop with an internal flag/Option so explicit close disables the destructor path.

🛠️ Suggested fix (guard Drop with Option)

 pub struct Session {
-	session: Arc<dyn SessionInner>,
+	session: Option<Arc<dyn SessionInner>>,
 }
 
 impl Session {
 	pub(super) fn new<S: web_transport_trait::Session>(session: S) -> Self {
 		Self {
-			session: Arc::new(session),
+			session: Some(Arc::new(session)),
 		}
 	}
 
 	/// Close the underlying transport session.
-	pub fn close(self, err: Error) {
-		self.session.close(err.to_code(), err.to_string().as_ref());
+	pub fn close(mut self, err: Error) {
+		if let Some(session) = self.session.take() {
+			session.close(err.to_code(), err.to_string().as_ref());
+		}
 	}
 
 	/// Block until the transport session is closed.
 	// TODO Remove the Result the next time we make a breaking change.
 	pub async fn closed(&self) -> Result<(), Error> {
-		let err = self.session.closed().await;
+		let err = self.session.as_ref().expect("session closed").closed().await;
 		Err(Error::Transport(err))
 	}
 }
 
 impl Drop for Session {
 	fn drop(&mut self) {
-		self.session.close(Error::Cancel.to_code(), "dropped");
+		if let Some(session) = self.session.take() {
+			session.close(Error::Cancel.to_code(), "dropped");
+		}
 	}
 }

🤖 Prompt for AI Agents

In `@rs/moq-lite/src/session.rs` around lines 46 - 49, The Drop impl for Session
currently always calls self.session.close(...), causing a double-close when
Session::close(self, ...) is called; modify the Session struct to hold an
internal guard (e.g., a bool like closed or an
Option<SessionInner>/Option<Transport>) that tracks whether the session has
already been closed, update Session::close (the consuming method) to set that
guard before returning, and change impl Drop for Session to check the guard and
only call the internal close logic if the session is not already closed; if
needed, factor the actual close implementation into a private fn
close_inner(&mut self, code, reason) so both Session::close and Drop can invoke
it without consuming self.