Repository: blackbeam/rust-mysql-simple (language: rust)

Issue:
`reset()` panics if called on borked `mysql::Conn`
Don't have an exact reproduction steps but this should be something like this:

1) Start target db
2) Create `mysql::Conn()` and make sure it's able to do some queries.
3) shutdown target db
4) Code that executed query on the mysql conn now Errors out.
5) In the Error handler invoke `conn.reset()`
This causes a panic with "incomplete connection" message in it.

That was first time I used the `reset()` function, I'll just stick to recreating the conn altogether - maybe that's expected usage, but found above a bit surprising.


Discussion / hints:
Hi.

This seem strange. I couldn't reproduce the error. Is it possible for you to get a backtrace?
Hello!

Apologies, issue reproduces in my environment 100% of the cases, although I am only using it against my real prod like scenario, i did not bother to produce mcve - sorry!

Few additional notes, not sure if they matter much but:
1) I am executing multi statement queries
2) this is fn that actually executes the queries:

```
pub fn exec_multi_statement_query(conn: &mut mysql::Conn, query: &str) -> anyhow::Result<()> {
    let result: anyhow::Result<()> = try {
        let mut result = conn.query_iter(query)?;
        while let Some(result_set) = result.iter() {
            for row in result_set {
                row?;
            }
        }
    };
    if result.is_err() {
        if let Err(e) = conn.query_drop("ROLLBACK") {
            // logging e here...
            conn.reset()?; // <<< this panics. but luckily is not necessary in my code.
        }
        result?;
    }
    Ok(())
}
```

below is the stack:
```
                               at /rustc/777bb86bcdbc568be7cff6eeeaaf81a89b4aa50b/library/core/src/option.rs:715:21
  21:     0x5635a79c5ff9 - mysql::conn::Conn::stream_mut::he637066580ae75eb
                               at /xyz/mysql-22.1.0/src/conn/mod.rs:283:9
  22:     0x5635a79cc71e - mysql::conn::Conn::reset_seq_id::hcac53cf284f25566
                               at /xyz/mysql-22.1.0/src/conn/mod.rs:785:9
  23:     0x5635a79cc980 - mysql::conn::Conn::write_command::hb22d69e0576dfd64
                               at /xyz/mysql-22.1.0/src/conn/mod.rs:806:9
  24:     0x5635a79ce631 - mysql::conn::Conn::_query::h9bfd4bde978c4c34
                               at /xyz/mysql-22.1.0/src/conn/mod.rs:961:9
  25:     0x5635a79cf498 - <mysql::conn::Conn as mysql::conn::queryable::Queryable>::query_iter::h15bf45012e4d9e59
                               at /xyz/mysql-22.1.0/src/conn/mod.rs:1145:20
  26:     0x5635a7643963 - <my code calls .reset()>
  ```

Is that sufficient?
> Is that sufficient?

Not sure. Is it somehow possible that `reset` is called multiple times?
sort of - there are multiple threads in parallel that are pumping data to database.

but on the same conn, I don't think so.
Actually looking at the code yes.
This may happen.

I only update the `conn` if the establishing of new conn has succeeded, if it did not it will use try to use the same conn, so my bad. 
Anyway, code assumes that the stream is always available, it's a bug. Thanks for report!

Fix patch:
diff --git a/src/conn/mod.rs b/src/conn/mod.rs
index d6b6221..7ce7147 100644
--- a/src/conn/mod.rs
+++ b/src/conn/mod.rs
@@ -160,8 +160,11 @@ struct ConnInner {
     opts: Opts,
     stream: Option<MySyncFramed<Stream>>,
     stmt_cache: StmtCache,
+
+    // TODO: clean this up
     server_version: Option<(u16, u16, u16)>,
     mariadb_server_version: Option<(u16, u16, u16)>,
+
     /// Last Ok packet, if any.
     ok_packet: Option<OkPacket<'static>>,
     capability_flags: CapabilityFlags,
@@ -354,7 +357,6 @@ impl Conn {
     }
 
     fn hard_reset(&mut self) -> Result<()> {
-        self.0.stream = None;
         self.0.stmt_cache.clear();
         self.0.capability_flags = CapabilityFlags::empty();
         self.0.status_flags = StatusFlags::empty();
@@ -1751,6 +1753,39 @@ mod test {
             handle.join().unwrap();
         }
 
+        #[test]
+        fn reset_does_work() {
+            let mut c = Conn::new(get_opts()).unwrap();
+            let cid = c.connection_id();
+            c.reset().unwrap();
+            match (c.0.server_version, c.0.mariadb_server_version) {
+                (Some(ref version), _) if *version > (5, 7, 3) => {
+                    assert_eq!(cid, c.connection_id());
+                }
+                (_, Some(ref version)) if *version >= (10, 2, 7) => {
+                    assert_eq!(cid, c.connection_id());
+                }
+                _ => assert_ne!(cid, c.connection_id()),
+            }
+        }
+
+        /// Library panics with "incomplete connection" in case of subsequent
+        /// failed calls to `reset` when the server is down.
+        /// (see [blackbeam/rust-mysql-simple#317][1]).
+        ///
+        /// [1]: https://github.com/blackbeam/rust-mysql-simple/issues/317
+        #[test]
+        fn issue_317() {
+            let mut c = Conn::new(get_opts()).unwrap();
+            c.0.opts = get_opts().tcp_port(55555).into();
+            let version = std::mem::replace(&mut c.0.server_version, Some((0, 0, 0)));
+            let mdbversion = std::mem::replace(&mut c.0.mariadb_server_version, Some((0, 0, 0)));
+            c.reset().unwrap_err();
+            c.0.server_version = version;
+            c.0.mariadb_server_version = mdbversion;
+            let _ = c.reset();
+        }
+
         #[test]
         fn should_drop_multi_result_set() {
             let opts = OptsBuilder::from_opts(get_opts()).db_name(Some("mysql"));


Test patch: