Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embedded start/check data race #5

Open
karalabe opened this issue Jun 29, 2018 · 3 comments
Open

Embedded start/check data race #5

karalabe opened this issue Jun 29, 2018 · 3 comments

Comments

@karalabe
Copy link
Contributor

If I start an embedded tor process, you are currently starting it and immediately checking control port status https://github.com/cretz/bine/blob/master/tor/tor.go#L142:

	err := tor.startProcess(ctx, conf)
	// Connect the controller
	if err == nil {
		err = tor.connectController(ctx, conf)
	}

this however is racey, because it's almost impossible for tor to start up fast enough for the control port to be open by the time you reach the check. If I explicitly add a couple second sleep before connect, everything works nicely, otherwise starting just fails with Error on start: dial tcp 127.0.0.1:9051: connect: connection refused.

@karalabe
Copy link
Contributor Author

Btw, I played around with this code a bit and your code seems to work fine with Tor 0.3.3, but the racey behavior appears with Tor 0.3.5. Probably they changed initialization order a bit so the new Tor gets to the control port later than the current stable one.

@cretz
Copy link
Owner

cretz commented Jun 29, 2018

True, suggestions? I am not sure the best cross-platform way to wait until a port is open by an external process before proceeding.

I can update connectController to leverage the context's doneness (I should do this anyways with net.DialContext+textproto.NewConn since textproto doesn't have its own DialContext) and keep retrying. Unfortunately, for this to be reasonable I might need to only retry on connection refused instead of other connection failure that would force the caller to wait until timeout.

I can also wait on some stdout output from the tor process I suppose, but I want the user to be able to control output verbosity.

I could also do this in embeddedProcess.start to have it sleep for a (configurable?) sec.

Suggestions?

@cretz
Copy link
Owner

cretz commented Jun 29, 2018

Ah, I bet this works with an 0 for control port because it waits for the control port file to be written. Maybe I should always use a control port file by default, expose a NoControlPortWriteFile bool, and apply the current auto-port-lookup-via-file logic to all situations?

papr8ka pushed a commit to papr8ka/bine that referenced this issue Sep 18, 2022
papr8ka pushed a commit to papr8ka/bine that referenced this issue Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants