search

poggit / libasynql

Asynchronous MySQL access library for PocketMine plugins.
https://poggit.github.io/libasynql
Apache License 2.0
132 stars 44 forks source link

Fix MysqlCredentials way of handling connection errors. #111

Closed Yexeed closed 8 months ago

Yexeed commented 8 months ago

It may seem weird if you look at the code, but mysqli->connect_error is still used. Hence, we are sure that we catch any type of mysqli error, whether it is thrown or not. (PHP's mysqli seem to bug this out: if you're doing first connection, an exception will be thrown. If it's a reconnect situation, no exception is thrown, you are able to check connect_error to decide if an error has happened.

Before PR: image (you're stuck with frozen main thread and the only way to get out is to CTRL-Z it)

After PR: image (an error is correctly catched, results into dataConnector being null, and infected plugin is able to check if connection is not successful)

SOF3 commented 8 months ago

how would this cause "frozen main thread"?

Yexeed commented 8 months ago

how would this cause "frozen main thread"?

Well, as far as I can tell, the mysqli_sql_exception is thrown but it is not in a try-catch block (here https://github.com/poggit/libasynql/blob/master/libasynql/src/poggit/libasynql/mysqli/MysqlCredentials.php#L115)

Any unhandled exceptions are crashing thread where it is thrown. This is the screenshot from a console when trying to initialize libasynql db upon enabling a plugin. As you can see, the thread crashes and server's main thread has frozen, refusing to load any plugins or stuff. Pressing CTRL-C only shows "Received signal interrupt" and still, nothing happens. Only after you CTRL+C it multiple times, or maybe CTRL-Z on linux, the process is killed. image

SOF3 commented 8 months ago

It seems that main thread doesn't freeze, it's just polling waitAll called by the plugin. Which is reasonable, because the queries are indeed not complete and have to be "waited".

SOF3 commented 8 months ago

I mean it doesn't directly cause the main thread to freeze, it's just working as expected.

Yexeed commented 8 months ago

it's just polling waitAll called by the plugin

Yup, thats the case, thanks for explanation