neo4j / neo4j-python-driver

Neo4j Bolt driver for Python
https://neo4j.com/docs/api/python-driver/current/
Other
898 stars 186 forks source link

Missing resource closure warnings #949

Closed solter closed 1 year ago

solter commented 1 year ago

Bug Report

When the driver is used after it has been closed, it may open up new resources. But the _closed state is not reset, and so deleting a driver with open resources may leave abandoned, open resources.

Example code

Here is an example of how this might occur:

import neo4j

driver = neo4j.GraphDatabase.driver(uri, (username, password))

driver.verify_connectivity()
print(f"{driver._closed}, {len(driver._pool.connections}") # False, 1

driver.close()
print(f"{driver._closed}, {len(driver._pool.connections}") # True, 0

# del driver # this would be safe now

driver.verify_connectivity()
print(f"{driver._closed}, {len(driver._pool.connections}") # True, 1

# driver has unclosed resources
del driver # should generate an unclosed resource warning, but does not

My Environment

Python Version: 3.11 Driver Version: 5.10.0 Server Version and Edition: Neo4j Desktop, Neo4j DBMS 5.9.0 Operating System: Windows 10

solter commented 1 year ago

I'm pretty sure this is due to the following:

Thus if resources are allocated (for example in the verify_connectivity method) after the driver has been closed, the _closed state is not reset.

Potential solutions:

  1. update the resource warning check to fire if any resources are allocated in the _pool object
    • should just be able to check the _pool since it is the only thing cleaned up by close()
    • instead of when the _closed flag is set
    • this is probably the most robust fix
      • assuming that it is known how to query to pool for open resources for any kind of pool
        • unclear if this is possible, since the type declaration states that _pool is of type any
  2. update the driver to disallow method calls that might open resources after it has been closed
    • could break existing code
    • would not recommend
  3. update the _closed flag so that it is reset to false any time a method is called that may allocate resources
    • probably the easiest fix
    • should be similarly robust as method 2, but slightly more error prone for future development
      • since any new methods on Driver or Driver subclasses would need to remember to reset the _close flag
robsdedude commented 1 year ago

Hello and thanks for reaching out :wave:

Well spotted. This is a weird quirk specific to the Python driver that we are already aware of. For Version 6.0 we planned to go with option 2 (choosing 6.0 as it's a breaking change). Why option 2? Because it's weird and surprising behavior that after closing a resource (here the driver) you can continue to use it. This might also result in us being limited on what we can and cannot do on driver closure if we don't want to break this. We consider using the driver after closure invalid usage of the driver. It's more an oversight than by design that it's currently working.