drogonframework / drogonframework/drogon
A couple of potential issues with mysql driver
- Dominant language
- C++
- Stars
- 14.3k
- Forks
- 1.4k
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 14
Description
Hi!
I am implementing async C++ driver for mysql on top of mariadb-connector-c myself, and i've hit 2 very nasty issues during my testing, so i thought i could share them with you.
Without further ado:
**First potential issue**:
`mysql_close` [called in connection destructor](https://github.com/drogonframework/drogon/blob/120aaf249dbd97a25757015754a5bb705d50b44d/orm_lib/src/mysql_impl/MysqlConnection.cc#L54) may block forever under some very rare yet possible circumstances.
Quoting the docs of `mysql_close_start`:
`
mysql_close() sends a COM_QUIT request to the server, though it does not wait for any reply.
Thus teoretically it can block (if the socket buffer is full), though in practise it is probably unlikely to occur frequently.
`
Turns out there is a pretty easy way to make the socket buffer full (and make it stay full):
1. take this code https://pastebin.com/raw/qGaqrNXU
2. start mariadb: `docker run -dit --name some-mariadb --network=host --env MARIADB_USER=mysql_close_blocks_forever --env MARIADB_PASSWORD=password --env MARIADB_ALLOW_EMPTY_ROOT_PASSWORD=1 mariadb`
3. Run the code
4. `docker pause` mariadb container when the code prompts for it
5. resume code execution - it hangs like this https://pastebin.com/raw/8LVXSPz9 until mariadb container is unpaused
I am lucky to have stackfull coroutines backing my async, so i can await in destructor and doing it like this
```
void NativeInterface::Close(MYSQL* mysql) && {
try {
socket_.RunToCompletion([mysql] { return mysql_close_start(mysql); },
[mysql](int mysql_event) {
return mysql_close_cont(mysql, mysql_event);
},
deadline_);
} catch (const std::exception& ex) {
// Us being here means we timed out on sending COM_QUIT.
// We must drive mysql_close_cont to completion, otherwise connection gets
// leaked, but mysql_close_cont won't go any further until socket is
// unblocked, so we `shutdown` the socket.
// https://jira.mariadb.org/browse/CONC-621
mariadb_cancel(mysql);
mysql_close_cont(mysql, 0);
LOG_WARNING() << "Failed to correctly close a connection, forcibly "
"shutting it down. Reason: "
<< ex.what();
}
}
```
seems to work, however i'm not sure how this could be implemented via callbacks.
**Second potential issue**:
Errors from `mysql_real_connect_cont` are unrecoverable prior to libmariadb3=3.3.4 (https://jira.mariadb.org/projects/CONC/issues/CONC-622).
There is an explanation and a repro in the issue, and TLDR is:
after trying to connect to a port that is not listening (thus making `mysql_real_connect_cont` fail, [this branch](https://github.com/drogonframework/drogon/blob/120aaf249dbd97a25757015754a5bb705d50b44d/orm_lib/src/mysql_impl/MysqlConnection.cc#L195) in the code), `mysql_close` invokes double free; not calling `mysql_close` leaks.
In my implementation in this case i'm leaking with some critical severity logs if linked against libmariadb < 3.3.4, and frankly i'm not sure what's the best solution here, since 3.3.4 is very recent and it seems too demanding to require it from users.
That's it, hope this helps to make an already great piece of OSS even better.
Contributor guide
Assessment
This issue has not been assessed yet.