Skip to content

Consistently check connection validity in AsyncMysqlConnection #9621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mszabo-wikia
Copy link
Contributor

@mszabo-wikia mszabo-wikia commented Jun 25, 2025

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

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 facebook#8678:
```hack
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');
    $async_conn->close();

    var_dump($async_conn->getSslCertCn());
}

async function func_async(AsyncMysqlConnection $asyncMysql, SQL\Query $query): Awaitable<void> {
    $query->toString__FOR_DEBUGGING_ONLY($asyncMysql);
    var_dump($asyncMysql->getSslCertCn());
    await $asyncMysql->queryf('SELECT %s', 'something');
}
```

and for `(get|is)Ssl.*`:
```hack
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 facebook#8678
@facebook-github-bot
Copy link
Contributor

@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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ Segmentation fault ] Using SQL\Query->toString__FOR_DEBUGGING_ONLY() causes a segmentation fault when a query is also running
2 participants