[tor-dev] Status report - Stream-RTT

[Damian Johnson]

Hi Robert, sorry about the delay. I couldn't sink the time a reply to
this thread deserved until now.

> -) When I wanted to check if a certain node is an exit node it took me some
> time to figure out that looking for an exit flag is not sufficient because some
> nodes are in fact exit nodes but don't have an exit flag.

Yup. It's unfortunate that tor decided to include an 'Exit' flag with
such an unintuitive meaning. You're not the first person to be
confused by it.

> One has to look at
> the nodes exit policy which is unaccessible by default because of
> microdescriptors. Maybe returning some meaningful message when one uses
> get_server_descriptor() and microdescriptors are enabled would help..?

Good idea! Done...

https://gitweb.torproject.org/stem.git/commitdiff/e78f1b7

> -) It is not safe to use extend_circuit in parallel for creating new circuits.
> I think this is not mentioned anywhere.

What kind of issue does that encounter? Is it a problem with stem's
thread safety or an issue on tor's side?

>> or would like a code review then let me
>> know.
>
> That would be awesome!

Few things I'm spotting offhand...

> self._lock.acquire()

Manual lock handling is risky. If anything within this block raises an
exception (and there's several points throughout your script where you
use Controller methods that can potentially raise errors) then the
lock won't be released.

The safer way of doing this is to use the 'with' keyword...

with self._lock:
  # do stuff

This is the same as...

try:
  self._lock.acquire()
  # do stuff
finally:
  self._lock.release()

> def read(self):
>   ...
>   return None

Not necessary. Methods return None by default.

> # pylint: disable-msg=R0902

You might want to look into pyflakes and pep8. I've found them to be
better static analysis tools.

> try:
>   controller = connect_port()
> except SocketError:
>   sys.stderr.write("ERROR: Couldn't connect to Tor.\n")
>   sys.exit(1)
> controller.authenticate()

Not quite. The connect_port() function never returns an exception.
Rather, if it fails to establish a control connection then it prints
the issue to stdout and returns None. Also, the connection it provides
is already authenticated.

This should instead be...

controller = connect_port()

if not controller:
  sys.exit(1)  # failed to get a control connenction


Cheers! -Damian