Fix for the SOCKS isolation test #48
Conversation
| @@ -190,6 +202,14 @@ - (void) socket:(GCDAsyncSocket *)sock didReadData:(NSData *)data withTag:(long) | |||
|
|
|||
| - (void) socketDidDisconnect:(GCDAsyncSocket *)sock withError:(NSError *)err { | |||
| NSLog(@"socketDidDisconnect %@", err); | |||
| if (err.code == 4) { // Sometimes the socket connects but cannot read data and keeps failing, I suspect the IP is rejected by the server | |||
| NSError *error = nil; | |||
| [((GCDAsyncProxySocket *) sock) initWithDelegate:self delegateQueue:dispatch_get_main_queue()]; | |||
There was a problem hiding this comment.
Hasn't sock already been initialized?
There was a problem hiding this comment.
Yes, but I found that if I didn't do that and just tried to reconnect there, it would keep trying and failing until the waitForExpectationsWithTimeout timed out. I couldn't create a new socket there since it would become nil as soon as it exited there.
Is an approach with socket variables that are more persistent more desirable?
There was a problem hiding this comment.
How about using an ivar?
|
I'll do that, I'm afk now but will take care of it by the weekend. |
|
This should do it. I wonder, do you prefer PRs with a single commit? |
|
Tests are still failing on Travis but I think it's because the timeout for the other test isn't long enough. I think this is good to merge and we could try to fix the other issue separately though. |
|
I've included an extra commit to address #49, but don't think this is an adequate fix since it now only builds and runs for iOS10 devices. Let me know if you want to merge without it and I'll roll it back. Wow, this is the first time I see |
rainwolf
force-pushed
the
master
branch
2 times, most recently
from
0b627db to
116ada7
Compare
|
I can't merge if it only runs on iOS 10. I need to support back to iOS 8, however, only building with Xcode 8 / iOS 10 SDK is ok. |
|
Also for using the xcode8 image on Travis we need to use xcodebuild / xcpretty because xctool is no longer maintained and doesn't work |
|
Do you want to merge without the last commit? Then you'd have support to iOS8 but it won't build on XCode 8, only on XCode 7.3. If you want, I'll undo my last commit and redo it after the merge. Closing this PR until there's a solution for XCode8 and iOS8 is also fine with me. |
|
Yeah it's rough because I need to do both Xcode 8 and iOS 8... I'll take a closer look soon. |
- bumped OpenSSL to 1.0.2i - ƒFixed compilation for XCode 8 by adding the host parameter for the i386 and x86_64 architectures - fixed unresolved symbols by removing checks for clock_gettime and getentropy from the configure scripts of libevent and Tor (hacky)
|
I made it work on XCode 8 and probably iOS 8, can confirm it works on iOS 9.3.1 at least. It seems iOS10 and Sierra added symbols I removed the checks for them from the configure scripts of libevent and Tor with a patch for each. There's probably a better way to fix this, but I don't know of it. |
|
Thank you for your work on this! |
rainwolf
force-pushed
the
master
branch
7 times, most recently
from
90330fa to
411ea86
Compare
|
Sweet! Did you report the patch to upstream Tor dev? |
The SOCKS isolation test kept producing SSL errors, both locally and on Travis, my workaround is to do a manual "verification".
In addition, it sometimes failed to read data, and retrying didn't help. I suspect the server rejected the IP-address, a workaround there is to reinitialize the socket with the same parameters, that seems to eventually enable the socket to connect and read data.
testSendingHUP keeps failing still, the callback never seems to get called, but the test does pass locally, both in XCode and with XCTool.