File Descriptors

[mmaker]

Currently txtorcon opens a couple o files without removing/closing them.

For example, when trying to fetch the .onion username, it uses a raw open, while a with statement should be used, also for preventing errors. Another more important bug concerning file descriptors left open, is the file for logging opened here[0] which writes a file in the current directory without letting the user configure the debug.

I would propose to use standard twisted.python.log for logging events, and use the with statement on hiddenservice's getattr. If that's ok for @meejah I would start wrtiting a patch :)

[0] https://github.com/mmaker/txtorcon/blob/master/txtorcon/torcontrolprotocol.py#L165

[meejah]

Yes, better use of logging would be great!

That torcontrollerfoo.log should probably be turned off by default, too, as it will leak a lot of information onto your harddrive (but is great for debugging when something mysteriously gets out of sync).

[mmaker]

f7d78c99fe619e39950ab5b72ff72dc1e51f6dc0 fixes the file descriptors stuff. Going to fix the logging issue :)

[meejah]

Are you sure that a file-descriptor leaks without the "with"? I would expect the file object to be collected immediately after the line with the "open()" in it. I'll read more about this tonight; thanks for the updates at the very least the whitespace fixes are good :)

A quick test seems to indicate that the file won't be leaked:

mike@pangea:~$ jobs -l
[1]+  9900 Stopped                 python
mike@pangea:~$ ls -l /proc/9900/fd
total 0
lrwx------ 1 mike mike 64 May 21 12:09 0 -> /dev/pts/9
lrwx------ 1 mike mike 64 May 21 12:09 1 -> /dev/pts/9
lrwx------ 1 mike mike 64 May 21 12:09 2 -> /dev/pts/9
mike@pangea:~$ fg
python

>>> f = open('/tmp/foo','w')
>>> f.write('foo')
>>> 
[1]+  Stopped                 python
mike@pangea:~$ ls -l /proc/9900/fd
total 0
lrwx------ 1 mike mike 64 May 21 12:09 0 -> /dev/pts/9
lrwx------ 1 mike mike 64 May 21 12:09 1 -> /dev/pts/9
lrwx------ 1 mike mike 64 May 21 12:09 2 -> /dev/pts/9
l-wx------ 1 mike mike 64 May 21 12:09 3 -> /tmp/foo
mike@pangea:~$ fg
python
f.close()
f.close()
>>> x = open('/tmp/foo','r').readlines()
>>> 
[1]+  Stopped                 python
mike@pangea:~$ ls -l /proc/9900/fd
total 0
lrwx------ 1 mike mike 64 May 21 12:09 0 -> /dev/pts/9
lrwx------ 1 mike mike 64 May 21 12:09 1 -> /dev/pts/9
lrwx------ 1 mike mike 64 May 21 12:09 2 -> /dev/pts/9
mike@pangea:~$ fg
python
x
x
['foo']
>>> 

[mmaker]

Yep, this works on cpython. But IMHO the with statement is still correct, for preventing errors and python interpreters different from cpython, for example pypy[0].

[0] http://doc.pypy.org/en/latest/cpython_differences.html#differences-related-to-garbage-collection-strategies

[meejah]

Ah, okay, thanks!

p.s. I merged the "with" stuff and whitespace diffs.

[meejah]

Please open a new request when you have a logging-improvements patch