Consistently check connection validity in AsyncMysqlConnection
The Squangle connection pointer wrapped by AsyncMysqlConnection may be nullptr if the connection was closed or is currently busy waiting for the result of an async query. Most code paths already call either verifyValidConnection() to raise an appropriate Hack exception or explicitly check for and handle a null backing connection, but Query::toString__FOR_DEBUGGING_ONLY() and the SSL-related getters from D33663743 do not, which can lead to segfaults.
Slightly simplified reproducer from #8678:
use namespace HH\Lib\SQL;
<<__EntryPoint>>
async function main_async(): Awaitable<void> {
// connection parameters as needed
$async_conn = await AsyncMysqlClient::connect('127.0.0.1', 3306, 'foo', 'root', 'pass');
concurrent {
await func_async($async_conn, new SQL\Query('SELECT %s', 'something'));
await func_async($async_conn, new SQL\Query('SELECT %s', 'something'));
}
}
async function func_async(AsyncMysqlConnection $asyncMysql, SQL\Query $query): Awaitable<void> {
$query->toString__FOR_DEBUGGING_ONLY($asyncMysql);
await $asyncMysql->queryf('SELECT %s', 'something');
}
and for (get|is)Ssl.*:
use namespace HH\Lib\SQL;
<<__EntryPoint>>
async function main_async(): Awaitable<void> {
$async_conn = await AsyncMysqlClient::connect('127.0.0.1', 3306, 'foo', 'root', 'wikia123456');
$async_conn->close();
var_dump($async_conn->getSslCertCn());
}
Call verifyValidConnection() in all these cases, as raising an appropriate exception (e.g. closed/busy connection) seems appropriate here.
Fixes #8678
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. (Because this pull request was imported automatically, there will not be any future comments.)
This code looks sensible to me, but I'm slightly nervous about changing C++ in core DB code when I have less context on the C++ components. I'm trying to find a reviewer.
Thanks! Happy to add tests for this if the CI has a MySQL instance available—I see ext_mysql has tests that look for connection parameters in MYSQL_TEST_* environment variables.
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing.
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D77272245. (Because this pull request was imported automatically, there will not be any future comments.)
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing.