connections are not closed automatically because pymssql keep a strong reference on all connections

[GoogleCodeExporter]

I am having an issue with the usage of the connection_object_list in _mssql.pyx.

This is a list that is used in msg_handler() in order to obtain the error 
messages.

The problem I have is the following. Since the global connection_object_list is 
keeping a strong reference on all connections, when a connection is no longer 
referenced, it does not get deleted and the socket to the database stays opened.

This situation can lead a program to have too many opened files if the close() 
method is not explicitely called (this is causing me some troubles with unit 
tests).

Could it be possible to consider using a weak ref list instead, so the 
connection would get deleted when no longer referenced?

Original issue reported on code.google.com by guillaum...@gmail.com on 8 Jan 2013 at 6:18

[GoogleCodeExporter]

Here is a suggested implementation of this idea.

Original comment by guillaum...@gmail.com on 8 Jan 2013 at 6:45

Attachments:

• 107-weakref.patch

[GoogleCodeExporter]

Here is an updated version of the patch.

Original comment by guillaum...@gmail.com on 8 Jan 2013 at 9:35

Attachments:

• 107-weakref.patch

[GoogleCodeExporter]

Thank you for this patch! At a glance, this looks good. I'll give it a more 
thorough look sometime, but it might take a while because I'm on vacation now 
for 2 weeks. 

Original comment by msabr...@gmail.com on 10 Aug 2013 at 8:16

• Changed state: Accepted
• Added labels: Leak

[GoogleCodeExporter]

Original comment by msabr...@gmail.com on 10 Aug 2013 at 8:19

• Added labels: Needstesting

[GoogleCodeExporter]

https://code.google.com/p/pymssql/issues/detail?id=88 appears to be the same 
issue, so I think that the patch here will probably allow us to close TWO bugs. 
:-)

Original comment by msabr...@gmail.com on 10 Aug 2013 at 8:51

[GoogleCodeExporter]

Original comment by msabr...@gmail.com on 10 Aug 2013 at 9:06

[GoogleCodeExporter]

Great! Thanks for reviewing the patch!

I have been running this patch in production since January, and so far it has 
been proven to be stable.

If there is anything to be changed in the patch, let me know!

Original comment by guillaum...@gmail.com on 11 Aug 2013 at 2:44

[GoogleCodeExporter]

From a quick experiment last night, it appears that this patch might not be 
needed after this commit:

https://code.google.com/p/pymssql/source/detail?r=54554641cc7e487b9e8d5aa63b7085
bc31a5f060

The above commit makes it so that connections actually get closed and removed 
from connection_object_list when they are out of scope.

Can you test it out?

Original comment by msabr...@gmail.com on 13 Aug 2013 at 4:57

[GoogleCodeExporter]

As I mentioned previously, I don't know whether this weakref change is 
necessary when connections deallocate properly, but I tweaked the earlier patch 
a bit for style (rename self.weakref to self._weakref to reflect its private 
nature) and to prevent a compilation error that I was getting with Cython 
0.19.1:

https://gist.github.com/msabramo/6223415

Will hold off on applying this to see if it's necessary. 

Original comment by msabr...@gmail.com on 14 Aug 2013 at 3:05

[GoogleCodeExporter]

Original comment by msabr...@gmail.com on 14 Aug 2013 at 3:29

• Changed state: Fixed