Amir's Changes to Test Connectivity #275
Conversation
| tcpReports = append(tcpReports, report) | ||
| }, | ||
| }) | ||
| ctx = getReportFromTrace(ctx, r, hostname) |
There was a problem hiding this comment.
Both of our codes have a race condition. We need a mutex to append to the DNS queries and TCP connections.
I was imagining a different function here, one that takes callbacks with the dns and tcp reports instead, and the function can then append to whatever we need. That way the function doesn't depend on the specific report format.
BTW, it shouldn't depend on r, because the scope is too broad. It only needs the dns and tcp reports.
There was a problem hiding this comment.
@fortuna yes the race condition issue stands. I was planning to do that next. I can reduce the dependency on the report format
| r.DNSQueries = append(r.DNSQueries, report) | ||
| }, | ||
| ConnectStart: func(network, addr string) { | ||
| connectStart = time.Now() |
There was a problem hiding this comment.
This is not the correct behavior. In Happy Eyeballs we can have overlapping connection attempts.
We need a map network, addr -> start time instead.
Co-authored-by: Vinicius Fortuna <[email protected]>
Co-authored-by: Vinicius Fortuna <[email protected]>
Co-authored-by: Vinicius Fortuna <[email protected]>
Co-authored-by: Vinicius Fortuna <[email protected]>
Co-authored-by: Vinicius Fortuna <[email protected]>
* feat: allow unencoded `userinfo` as per SIP002 * Move shadowsocks config tests to `shadowsocks_test.go` and clean up. * Review comment. * Add test case for invalid cipher info (e.g. missin colon). * Remove unnecessary prefix in test.
|
Rebasing this branch on the top main has cluttered this PR. I am going to make a new PR based on top of main to avoid the clutter here. |
No description provided.