Skip to content
This repository has been archived by the owner on Mar 2, 2022. It is now read-only.

Remove DisableDebuggerAttachment option #83

Merged
merged 1 commit into from
Feb 11, 2018
Merged

Conversation

DonnchaC
Copy link
Member

@DonnchaC DonnchaC commented Feb 5, 2018

DisableDebuggerAttachment was added in 839fc5a but it doesn't work when applied to an already running Tor (#80). This option can only be set before Tor is launched. I'm removing it for now as I don't see why it is needed when running the bwscanner.

Resolves #80.

DisableDebuggerAttachment was added in 839fc5a but it doesn't work when applied to an already running Tor (#80). This option can only be set before Tor is launched. I'm removing it for now as I don't see why it is needed when running the bwscanner.
@coveralls
Copy link

coveralls commented Feb 6, 2018

Coverage Status

Coverage remained the same at 64.361% when pulling 0ec9bca on remove-disable-debugger into 839fc5a on develop.

@juga0
Copy link
Collaborator

juga0 commented Feb 6, 2018

According to this line https://github.com/TheTorProject/bwscanner/blob/develop/bwscanner/attacher.py#L174, getting first the tor state and then upgrading the configuration options should avoid the error Transition not allowed. So, it seems that getting first the tor state doesn't avoid the race condition.
Is there a way we could change the options before tor get launched?.
Also, why the error Transition not allowed would happen only with the option DisableDebuggerAttachment and not the others?.

I'm removing it for now as I don't see why it is needed when running the bwscanner.

According to @teor2345 #55 (comment), it was added because:

(Those last two are from chutney, see https://gitweb.torproject.org/chutney.git/tree/torrc_templates/common.i )

@ln5
Copy link

ln5 commented Feb 6, 2018

Also, why the error Transition not allowed would happen only with the option DisableDebuggerAttachment and not the others?.

Some options affect things which cannot be changed without a restart, see options_transition_allowed() [tor/src/or/config.c] for more options of that type.

@juga0
Copy link
Collaborator

juga0 commented Feb 6, 2018

I see, ok. Still i'll see if we can avoid the error by updating the options before tor gets launched.

@juga0
Copy link
Collaborator

juga0 commented Feb 11, 2018

Let's remove then the option for now, once #85 is merged we can see whether we should add back this option.

@juga0 juga0 merged commit 80d0285 into develop Feb 11, 2018
@juga0 juga0 deleted the remove-disable-debugger branch March 20, 2018 17:27
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.

4 participants