Possible bug in pool

[hwchen]

hello, I came across a bug in the pool implementation in clickhouse-rs, which after patching, the author mentioned that he drew from mysql_async pool implementation and you might have the same issue.

So, I'm linking the issue (which also has a link to the patch). I'm pretty sure I diagnosed the problem correctly and patched correctly, since the errors disappeared. But would be helpful if you could look over it also.

suharev7/clickhouse-rs#23

As a very general description for here, it looks like checking the new connections for readiness and then moving them to the ready/idle queue needs to hold the lock longer to avoid a race condition.

[hwchen]

Also, I'm happy to submit a patch if needed.

[hgmich]

I'm able to reproduce the same bug by load-testing this sample warp webapp I made with ab using fairly large concurrent request values (on my machine, -c 40 or so does the trick).

[blackbeam]

Hi, and thanks for your work.
I'll fix this in a few days.

[hgmich]

I made a conservative fix which just wraps the entire futures handler in a single lock acquisition and keeps a buffer of returned connections to process once the lock is yielded here:
https://github.com/holmesmr/mysql_async/commit/a81ba35407878785d1c7ba09175f52783474faf2
This may or may not be desirable, but it does at least seem to fix this bug.

[blackbeam]

@holmesmr, this should do the trick. Could you pleas file a PR?

[hgmich]

Done!

[blackbeam]

Fix published as v0.17.2.
Thanks!