code in ConnectionContainer.all_dead conflicts with Pool.putconn about momoko HOT 4 CLOSED

Looks like you are correct. How did you spot it? Do you have a minimal reproduction code that can be put to unittest?

from momoko.

I just looked into the unittest code. I guess we could spot this issue by adding a line follows L816 in test_abort_waiting_queue():

self.assertIsInstance(f2.exception(), momoko.Pool.DatabaseNotAvailable)

Not tested yet. Do you guess the same way?

from momoko.

But with the execution order of L478 and L479 in Pool._operate:

                if not keep:
                    self.putconn(conn)
                future.set_result(result)

I guess the second db query could get the only connection and issue the query command successfully. Then the testcase disconnects db by send SIGTERM to tcproxy. I consider it highly likely that the second db query could finish before the connection disconnect. Which would result in altered version of test_abort_waiting_queue()'s failure.
Maybe we need a better solution in unittest.

from momoko.

You are right both ways - I've changed the code to set the result first to let a done callback in the unittest to kill the connections. Then the assertion work (after fixing the original bug of course).

For the test with proxy, however the test will not raise DatabaseNotAvailable, because restarting proxy will not immediately mark the connection object as closed. I guess we'll have to live with it for now.

See #142. What do you think?

from momoko.