Handle or forbid putting of closed connections into a pool
Totktonada opened this issue · 8 comments
Case: a connection is acquired from a pool, then it is closed with <connection object>:close() and then it is put back to the pool.
Note: the code uses <connection object>.usable to mark closed connections or ones that was put back to a pool.
The documentation does not state that putting of closed connections is forbidden. The code don't give an error in the case and even correctly calculates that one more connection can be acquired (<pool object>.queue:put(nil) under hood). However this connection has GC callback that will close underlying raw connection and will allow to acquire one more from the pool: here the math fails.
(1) We can remove the GC callback and close the underlying connection at putting a closed connection to a pool.
(2) We however can give an error in the case: when a user attempt to put a connection with <connection object>.usable == false field, do nothing and give an error. We do not distingush between closed and released (put to a pool) connections: both have <connection object>.usable set to false. If we'll forbid to put closed connections, then we'll also forbid to put a connection twice. This looks as good protection from mistakes.
If we really need to put a manually closed connection to a pool, let's introduce one more flag to distinguish closed and released (put to a pool) connections.
I propose to implement the approach (2).
NB: Don't forget to add a test to verify the new behaviour.
May be related to TNT-24.
Alternative: Just remove close method from a connection from a pool.
Alternative: Just remove
closemethod from a connection from a pool.
Backward compatibility will be broken (and strange decision, IMHO). Now the behavior of the connection in the case of "close" is close to the same as in the "JDBC Connection Pool".
Backward compatibility will be broken
I disagree. It does not work correctly now.
Backward compatibility will be broken
I disagree. It does not work correctly now.
Why? As far as I remember, I can take a connection from the pool, close it and put it in the pool now. And this will be a valid code.
Backward compatibility will be broken
I disagree. It does not work correctly now.
Why? As far as I remember, I can take a connection from the pool, close it and put it in the pool now. And this will be a valid code.
So why this issue exists?
Backward compatibility will be broken
I disagree. It does not work correctly now.
Why? As far as I remember, I can take a connection from the pool, close it and put it in the pool now. And this will be a valid code.
So why this issue exists?
After some discussion, we decided to leave the close method for connection from the pool and add an exception throwing from the pool: put () method in case the connection has been closed earlier (close is synonymous with put in this context).
May be related to TNT-24.
The code in the internal issue does not use :close() (just pool:get(), conn:execute(), pool:put()), so the issue does not look related.