-
Notifications
You must be signed in to change notification settings - Fork 54
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
fix(test): tor transport test fails with SIGSEGV on macOS M1 #1105
Conversation
@@ -45,13 +45,12 @@ template commonTransportTest*(prov: TransportProvider, ma1: string, ma2: string | |||
|
|||
await conn.close() #for some protocols, closing requires actively reading, so we must close here | |||
|
|||
await handlerWait.wait(1.seconds) # when no issues will not wait that long! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when no issues will not wait that long
Is that still true when doing this before awaiting allFuturesThrowing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it's not necessary anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But not sure how this is related to the allFuturesThrowing
. I think it is just waiting for the acceptHandler
to finish and the conn
to be closed.
@@ -72,13 +71,14 @@ template commonTransportTest*(prov: TransportProvider, ma1: string, ma2: string | |||
|
|||
await conn.close() #for some protocols, closing requires actively reading, so we must close here | |||
|
|||
check string.fromBytes(msg) == "Hello!" | |||
await handlerWait.wait(1.seconds) # when no issues will not wait that long! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
Why does this fix the issue on MacOS M1 specifically?
startFut = stub.start(torServer) | ||
asyncTeardown: | ||
await startFut.cancelAndWait() | ||
await stub.stop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the changes from line 28-40 effect the described behaviour or is this general refactoring?
Adding more context, this error happened randomly when running the test alone and all the time when running My laptop was generally slow, and after investigating more, I found that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you.
more context: this is necessary for #1099 (see description)
Let's open an issue regarding investigating this error further.
Now that I know the problem was due to some unusually big memory consumption on my laptop, I'm not sure it's worth it to merge this hack. I did it cause I didn't know any better, but closing qemu and restarting the laptop fixed the problem. |
Thanks for the investigation. Closing this since the main problem this PR addressed is not occurring anymore. |
When running the
testtortransport
locally on macOS, it fails with the following error: