context managers used to allocate resources can be abused - a solution

Hello, while I was implementing a connection pool i've noticed a pitfall of our beloved with statements: with pool.get_connection() as conn: ....conn.execute(...) conn.execute(...) # the connection has been returned to the pool and does not belong to the user! a proposed solution: with protected_context(pool.get_connection()) as conn: ....conn.execute(...) conn.execute(...) # raises OutOfContextError() with protected_context(file("/tmp/bla.txt", "w")) as f: ....file.write("blo") file.write("blu") # raises OutOfContextError() the solution is to basically proxy all methods to the real object until the context ends and then the proxy expires. what do you think about adding it to contextlib? Thanks, Alon Horev

On Sat, Dec 17, 2011 at 11:00 AM, Alon Horev <alon@horev.net> wrote:
That's a bug in the connection pool implementation. If the underlying connection has been returned to the pool, the proxy returned by the context manager shouldn't work any more. Try the above code structure with a file object and it will give you "IO operation on closed file". Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On 12/16/2011 9:07 PM, Nick Coghlan wrote:
Good catch!
I believe Nick is saying that conn.__exit__ should close the connection and release the resource, that being the point of with statements and context managers. Can you open a tracker issue?
Try the above code structure with a file object and it will give you "IO operation on closed file".
Because file_object.__exit__ closes the connection to the OS resource. -- Terry Jan Reedy

On Sun, Dec 18, 2011 at 1:23 PM, Terry Reedy <tjreedy@udel.edu> wrote:
No, that's not what I'm saying at all - the whole purpose of using a connection pool is that you *don't* release the underlying OS resource when you return a resource to the pool. What I'm saying is that if a pool supports the context management protocol for *temporary* access to one of the pool resources, then *that* context manager is responsible for ensuring that the resource reference returned is correctly invalidated in __exit__ (just as closing a file object prevents further IO operations). I'm not aware of any such pools in the stdlib (multiprocessing has some pool APIs, but I don't believe they support the context management protocol for pooled resources), so I'm suggesting that the only problem here is in the design of the OP's own pool API. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On Sat, Dec 17, 2011 at 11:00 AM, Alon Horev <alon@horev.net> wrote:
That's a bug in the connection pool implementation. If the underlying connection has been returned to the pool, the proxy returned by the context manager shouldn't work any more. Try the above code structure with a file object and it will give you "IO operation on closed file". Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On 12/16/2011 9:07 PM, Nick Coghlan wrote:
Good catch!
I believe Nick is saying that conn.__exit__ should close the connection and release the resource, that being the point of with statements and context managers. Can you open a tracker issue?
Try the above code structure with a file object and it will give you "IO operation on closed file".
Because file_object.__exit__ closes the connection to the OS resource. -- Terry Jan Reedy

On Sun, Dec 18, 2011 at 1:23 PM, Terry Reedy <tjreedy@udel.edu> wrote:
No, that's not what I'm saying at all - the whole purpose of using a connection pool is that you *don't* release the underlying OS resource when you return a resource to the pool. What I'm saying is that if a pool supports the context management protocol for *temporary* access to one of the pool resources, then *that* context manager is responsible for ensuring that the resource reference returned is correctly invalidated in __exit__ (just as closing a file object prevents further IO operations). I'm not aware of any such pools in the stdlib (multiprocessing has some pool APIs, but I don't believe they support the context management protocol for pooled resources), so I'm suggesting that the only problem here is in the design of the OP's own pool API. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
participants (3)
-
Alon Horev
-
Nick Coghlan
-
Terry Reedy