Memory leak fix & more sanitization in tests

[pcantrell]

Fixes #268.

Adds a basic check for leaked Service (and thus Resource) instances to the test suite.

Enables assorted Xcode sanitization options in tests.

Fixes several unimportant but now-detected leaks & thread issues in the tests.

[jordanpwood]

Hey @pcantrell! Thanks so much for jumping on this. I'm planning to release a new version of my app this week, and I had included 1.4.0 in it. Would you recommend using memory_leak, or do you plan on getting a new version of Siesta released shortly (Travis issues?)? Is there anything I can do to help?

[jordanpwood]

I'm trying to setup Siesta so I can build and run the tests on my local machine, but I'm unable to get Carthage to compile Nocilla, I'm getting the following error:

User defaults from command line:
    IDEDerivedDataPathOverride = /Users/woodj/Library/Caches/org.carthage.CarthageKit/DerivedData/9.4.1_9F2000/Nocilla/bd7ec7caa0576f08c00bbbf993a9204f93be16e3

Build settings from command line:
    CARTHAGE = YES
    CODE_SIGN_IDENTITY = 
    CODE_SIGNING_REQUIRED = NO
    ONLY_ACTIVE_ARCH = NO
    SDKROOT = appletvsimulator11.4

xcodebuild: error: Unable to find a destination matching the provided destination specifier:
        { platform:tvOS Simulator, id:ECE73D0A-02FC-4B82-A5CA-FF2A9EBD7C09 }

    Ineligible destinations for the "Nocilla tvOS" scheme:
        { platform:tvOS, id:dvtdevice-DVTiOSDevicePlaceholder-appletvos:placeholder, name:Generic tvOS Device }
        { platform:tvOS Simulator, id:dvtdevice-DVTiOSDeviceSimulatorPlaceholder-appletvsimulator:placeholder, name:Generic tvOS Simulator Device }

If I'm reading that right, it wants a tvOS simulator which is version 11.4, but since I'm not yet on Xcode 10, I only have access to devices up to 11.3. Any suggestions on how to fix?

[pcantrell]

I'm planning to release a new version of my app this week, and I had included 1.4.0 in it. Would you recommend using memory_leak, or do you plan on getting a new version of Siesta released shortly (Travis issues?)? Is there anything I can do to help?

I expect that the memory_leak branch should be safe, but I’d love to see it pass CI before I say that with confidence. I’d happily welcome any additional detective work on this!

This PR’s new leak check in the tests is flagging leaks with the Alamofire provider. The problem is puzzling, and I haven't yet figured out whether it’s real or whether Alamofire just delays cleanup of its requests in a way that trips the test but is actually harmless.

If you want to take a look and puzzle over it, run the tests with the env var Siesta_TestMultipleNetworkProviders set to something nonempty. (I do this locally by manually changing the value to 1 in Edit Scheme → Test → Arguments.) You’ll know the flag is working if you see >1000 tests run instead of the usual 300-something.

You can see the AF issue in the Siesta macOS target, which runs much faster than the others because it doesn’t require a simulator.

The check that’s failing just keeps a weak ref to each test case’s Service, then checks that it’s nil at the end of the test. (This also catches Resource and Request leaks, since both of those ultimately retain their associated Service.) The interesting detail: it looks like each test case’s service gets cleaned up during the next test case. My suspicion is that AF does an additional something or other with each request that Siesta just isn’t waiting for.

I'm trying to setup Siesta so I can build and run the tests on my local machine, but I'm unable to get Carthage to compile Nocilla, I'm getting the following error: … If I'm reading that right, it wants a tvOS simulator which is version 11.4, but since I'm not yet on Xcode 10, I only have access to devices up to 11.3. Any suggestions on how to fix?

Could you open a separate issue for this? Then we can tag in the kind person who contributed the tvOS support.

[pcantrell]

Update: my current working theory is that AF is relying on autorelease for cleanup, and the autorelease pool doesn’t get drained until after my afterEach block that checks for leaks.

[pcantrell]

Yes, confirmed: at least some of the tests pass with Alamofire if each spec is wrapped in autoreleasepool { … }.

The problem now is that AFAICT, Quick doesn’t provide any sort of aroundEach facility that we could use to automatically wrap each test in an autoreleasepool { … }, and ARC doesn’t allow us to manually create and drain an autorelease pool in a way that Quick’s beforeEach and afterEach would support.

I have to dash off, and will be on other tasks most of the rest of the day. @jordanpwood, if you have some time today, you could help by investigating whether it’s possible to control autorelease pools with Quick. We need some way to make sure that each spec created during this call gets wrapped in its own pool, and that the pool is drained before the leak-checking block runs. The solution I’d want, but don’t know if Quick supports, would look like this:

aroundEach
    {
    spec in
    autoreleasepool
        { spec() } // (1)
    }

afterEach
    {
    // (2) …check for leaks here…
    }

…so that the order of execution is:

// (1) pool created
// spec runs
// (1) pool drained
// (2) leak check runs
// (1) pool created
// spec runs
// (1) pool drained
// (2) leak check runs
etc.

[pcantrell]

Trying a Quick fix (pun intended) for the Alamofire tests. If it passes CI, we’ll see if Quick wants to accept the new feature….

[jordanpwood]

It's almost passing the tests, are those leaked services real, or is it more test artifacts?

[pcantrell]

Phew, passed!

It was test artifacts, it seems — though establishing that and working around them took some finagling. The general flavor of all these leak-checking issues is that between Siesta, URLSession, and Alamofire, there’s a lot of deferred cleanup, and some of it is timing-sensitive. I hacked around it by adding a very forgive progressive delay-and-retry loop before we confirm the leak.

The good news is that the wait-and-rety appears to also take care of the nasty Travis issues that the unpleasant DelayAfterEachSpec flag was working around. Appears to. Without that delay, the specs run a bit faster on CI.