Skip to content

Connection._cleanup not being called when the connection is dropped #524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ioistired opened this issue Jan 10, 2020 · 7 comments
Closed

Comments

@ioistired
Copy link
Contributor

This issue is continued from comments on #421 and #283.

Problem steps

Run the following code:

#!/usr/bin/env python3

import asyncpg
import asyncio

class MyConnection(asyncpg.Connection):
	def _cleanup(self):
		print(1)
		return super()._cleanup()

async def amain():
	conn = await asyncpg.connect(connection_class=MyConnection)
	await asyncio.sleep(float('inf'))

if __name__ == '__main__':
	asyncio.run(amain())

Then restart postgres.

Expected results

"1" is printed.

Actual results

Nothing is printed.

System info

  • asyncpg version: commit 851d586
  • PostgreSQL version: 12.1
  • Python version: CPython 3.8.1
  • Platform: Arch Linux
  • Do you use pgbouncer?: No
  • Did you install asyncpg with pip?: No
  • If you built asyncpg locally, which version of Cython did you use?: 0.29.14
  • Can the issue be reproduced under both asyncio and
    uvloop?
    : Yes
@elprans
Copy link
Member

elprans commented Jan 10, 2020

Well, the code is clearly there:

cdef _on_connection_lost(self, exc):
if self.closing:
# The connection was lost because
# Protocol.close() was called
if self.waiter is not None and not self.waiter.done():
if exc is None:
self.waiter.set_result(None)
else:
self.waiter.set_exception(exc)
self.waiter = None
else:
# The connection was lost because it was
# terminated or due to another error;
# Throw an error in any awaiting waiter.
self.closing = True
# Cleanup the connection resources, including, possibly,
# releasing the pool holder.
con = self.get_connection()
if con is not None:
con._cleanup()
self._handle_waiter_on_connection_lost(exc)

So there are two options: 1) Either asyncio is not calling connection_lost, or 2) You don't keep a live reference to the connection so it gets garbage collected before _cleanup() is called.

@ioistired
Copy link
Contributor Author

How is (2) possible? amain() runs forever and does not del conn.

@elprans
Copy link
Member

elprans commented Jan 10, 2020

Well, your particular example is broken, since the amain task is never woken up.

@ioistired
Copy link
Contributor Author

ioistired commented Jan 10, 2020

Uhh… hm. This code:

#!/usr/bin/env python3

import asyncpg
import asyncio

cleanup_ran = False

class MyConnection(asyncpg.Connection):
	def _cleanup(self):
		global cleanup_ran
		cleanup_ran = True
		return super()._cleanup()

async def amain():
	conn = await asyncpg.connect(connection_class=MyConnection)
	while True:
		await asyncio.sleep(1)
		if conn.is_closed():
			print(cleanup_ran)
			break

if __name__ == '__main__':
	asyncio.run(amain())

prints True after restarting postgres. I see.

@ioistired
Copy link
Contributor Author

@elprans this code:

#!/usr/bin/env python3

import asyncpg
import asyncio

async def amain():
	conn = await asyncpg.connect()
	cleanup_ran = False
	def close_listener(_):
		nonlocal cleanup_ran
		cleanup_ran = True
	conn.add_close_listener(close_listener)
	while await asyncio.sleep(1, True):
		if conn.is_closed():
			print(cleanup_ran)

if __name__ == '__main__':
	asyncio.run(amain())

when run against my fork prints False every second after restarting postgres. Any clues as to why?

@elprans
Copy link
Member

elprans commented Jan 10, 2020

Put prints into def connection_lost() and def _on_connection_lost() right before it is calling _cleanup() to see where the call chain gets broken.

@ioistired
Copy link
Contributor Author

Oh hm… I was using pip install -e . which wasn't rebuilding the cython code. Running make causes the above code to print True. Thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants