Unregister from the reactor when connection is going to be closed.

[smira]

Otherwise we still may have incoming events from the socket, but there's no handler for them (self.pollable() becomes None).

[smira]

This bug is reproducible randomly, when having several concurrent connections to PostgreSQL and closing them

[wulczer]

I see how this can be happening, still working on a test case that will reproduce in in 100% of the cases.

Could you try with Twisted HEAD? Specifically this bug: http://twistedmatrix.com/trac/ticket/4539 has caused some problems in txpostgres, I wonder if a fixed version would also fix this issue. Probably not, but maybe worth a try.

The patches look good and I'll pull them shortly, although I want to have a test case ready first.

Thanks!

[smira]

Thanks for looking on that, actually I was using pretty new version of Twisted, I'm almost always following trunk. It is 2 weeks old, https://github.com/smira/twisted/.

[wulczer]

Oh, ok, thanks. Should have this wrapped up before the end of the weekend.

[wulczer]

Ugh, that took a while... I didn't manage to reproduce your issue, but while I was investigating this one, another bug surfaced: #6.

I committed a partial fix for it and a side effect is that pollable() will no longer be set to None after the connection is closed. If you're using epoll you might be affected by #6 but perhaps this issue got fixed by my last change. Could you try again?

[smira]

It took me a while to do testing and fixes with the new version, but it seems to be more stable than previous ones.

Summary of changes:

1. Don't close already closed connection, it is safe now to call close() twice.
2. In doRead/doWrite check for pollable() state: I had races under heavy tests (original change)
3. Reworked PollingMixin.poll() into two separate methods: one that generates and returns Deferred and another that does psycopg2 poll and processes its results. No more Deferreds generated without consumer (like from doRead called on Connection when waiting for NOTIFY), no more 'Unhandled error in Deferred' on close.

[wulczer]

Thanks for that!

I pushed the changes from 1) and 2) + a unit test.

I like the idea of 3) as I just noticed that closing the connection while listening for NOTIFY generates these annoying "unhandled error" messages.

I think I'll follow your suggestion and split poll() into two methods. I try to avoid Python asserts in code, so I might change that into a ValueError. I guess that since you call _poll() only from _continue_polling() I'm just going to inline it.

Oh, and the method name will have to change, since I try to follow Twisted's standard of using camel case (even though I hate it).

Thanks again and I'll try to push the last fix faster then the previous time :)

[wulczer]

I just pushed the split of poll() into poll() and continuePolling(). I hope this will finally resolve this issue.

Thank you for your analysis and patches, I think things are in a quite better shape now.

[oberstet]

I did some testing with

https://github.com/oberstet/txpostgres/blob/testing/testing/txpostgres_error.py

2 cases:

a) correct database password, should fail on "select * from nonexistent"
b) incorrect password

The latest
d13f8ff

give me tracebacks on both:

https://gist.github.com/1353252

The latest
https://github.com/smira/txpostgres/commit/e8571b02dced034a5289ef8cc4f4ea3e8d02d814

gives me traceback only on b), and works for a):

https://gist.github.com/1353259

[smira]

Thanks a lot, all my tests are working fine right now!

I'm closing this pull request, I have one more change related to stack overflow under certain circumstances, but that deserves new pull request )

[wulczer]

Great to hear that your testing proved fine now!

Don't hesitate to open a separate issue for the stack overflow. Thanks!