Code review comment for lp://staging/~diego-fmpwizard/drizzle/bug-436685

Hi!

Good catch on the bug. This was a bit of a hack to force the closing of the socket until shutdown could be cleaned up some more and we can remove oldlibdrizzle entirely and use the new libdrizzle. :)

Two things:

* I'm not sure if we need to move the tmp->client->close(); call inside the lock, I don't think that gets use anything

* The vio.cc changes are not needed (and that last line does nothing). At the top of drizzleclient_vio_delete vio is checked to be sure it's not NULL, so your != NULL check would be an extremely rare race condition, if at all (since the op right above would dereference vio for vio->read_buffer).

So, I would definitely revert the vio.cc change, and test to see if the drizzled.cc changes are really needed.

Thanks!
-Eric