Skip to content
This repository has been archived by the owner on May 4, 2021. It is now read-only.

fix: globals: Fetch descriptors early #358

Closed

Conversation

juga0
Copy link
Contributor

@juga0 juga0 commented Jun 16, 2019

So that sbws detect early changes in the relay descriptors.

Closes: #30733. Bugfix v0.2.0.

So that sbws detect early changes in the relay descriptors.

Closes: #30733. Bugfix v0.2.0.
sbws/globals.py Outdated
# Tor doesn't need all descriptors to function. In particular...
#
# * Tor no longer downloads server descriptors by default, opting
# for microdescriptors instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is wrong, because we set UseMicrodescriptors 0 above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it makes sense to have it in the context of stem, not sbws.
Fixup removed the context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/context/comment/

#
# * If you aren't actively using Tor as a client then Tor will
# eventually stop downloading descriptor information altogether
# to relieve load on the network.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good reason for us to use FetchUselessDescriptors: we don't want Tor going idle.

@juga0
Copy link
Contributor Author

juga0 commented Jun 17, 2019

This comment is also wrong:

"To download descriptors regardless of if they're needed by the
Tor process or not set..."

Replaced with actual man comment in the option.

Edit: typos

@teor2345
Copy link
Contributor

This code contains a race condition: new_relays_dict is modified while its keys are being iterated.
Python 3.8 will warn when this happens, maybe older versions do too.

I opened https://trac.torproject.org/projects/tor/ticket/30908 for this issue

@juga0
Copy link
Contributor Author

juga0 commented Jun 17, 2019

This code contains a race condition: new_relays_dict is modified while its keys are being iterated.
Python 3.8 will warn when this happens, maybe older versions do too.

Hmm, i'm not iterating new_relays_dict but relays. Are you sure about this?, do you have a log with python 3.8?.

To avoid the race condition, take a copy of the keys:
if r.fingerprint in list(new_relays_dict.keys()):

That's a condition, not a loop.

See this stem commit for a similar bug and fix:
https://gitweb.torproject.org/stem.git/commit/?id=b5aecb743d33db1a6378d59792d8e57305b6c6f2

It seems to me a different case

@teor2345
Copy link
Contributor

It seems to me a different case

You're right, I closed the ticket as "not a bug".

@juga0
Copy link
Contributor Author

juga0 commented Jan 26, 2020

Closing in favor of #362

@juga0 juga0 closed this Jan 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants