expect()ing the Unexpected

Our tests were crashing. They ran fine individually, but when run as a group, certain tests sometimes failed with a spectacular memory access error.

After experimenting with skipping some of the tests, I was able to narrow it down to tests that ran immediately after some database calls. (This was a mobile project for iOS, and we were using Realm.)

Here is the code that ultimately led to the crash. See if you can spot the bug.


- (void)overrideStoredHistoryValueOn:(NSDate *)endedOn settingItTo:(NSUInteger)value forType:(PRJHistoryRecordType)recordType {
     NSPredicate *predicate = [NSPredicate predicateWithFormat:@"endedOn == %@ AND type == %@", endedOn, @(recordType)];

     DDLogVerbose(@"Start transaction to override history value for %@", endedOn);
     RLMRealm *realm = [RLMRealm defaultRealm];
     [realm transactionWithBlock:^{
         DailyHistoryRecord *record = [[DailyHistoryRecord objectsWithPredicate:predicate] firstObject];
         expect(record).toNot.beNil();
         record.point = value;
     }];
     DDLogVerbose(@"Done with transaction to override history value for %@", endedOn);
}

Start a transaction, find the record for the test data we added during the test setup (and expect it to exist), update the data point for that day, and finish the transaction. Nothing too surprising.

While the root cause was still unclear, this gave me a lot more focus—I could limit everything to two specific tests, which meant reproducing the issue with less delays.

I tried commenting out the transaction block. It no longer crashed, but the test failed (no surprise—we weren’t actually updating things anymore). Then, I tried replacing the expect call with a BOOL, indicating whether the record was found, and expecting that to be true. It no longer crashed. Hmmmm.


 __block BOOL recordFound = NO;
 [realm transactionWithBlock:^{
     DailyHistoryRecord *record = [[DailyHistoryRecord objectsInRealm:realm withPredicate:predicate] firstObject];
     if (record) {
         record.point = value;
         recordFound = YES;
     }
 }];
 expect(recordFound).to.beTruthy();

Digging into the internals of Realm and Expecta made the root cause clearer. Since we were operating within a database transaction, the result wasn’t just another record object—it was a live handle into database internals and the results from our query, and Realm expected it to go away after leaving the transaction block. It was even marked __unsafe_unretained inside the C++ / Objective C mapping for Realm:

static inline void RLMSetValue(__unsafe_unretained RLMObjectBase *const obj, NSUInteger colIndex, long long val) {
    RLMVerifyInWriteTransaction(obj);
    obj->_row.set_int(colIndex, val);
}

Unfortunately, expect() didn’t just fail the test if the record wasn’t found. It was also kept a reference to it, wrapped up inside a new block, along with some other test info. (Probably for reporting failures.)

This meant that the test would continue along and ultimately pass, but left a stale handle to a database object floating around. It would be cleaned up, but that could overlap with when the next test had already started and emptied the database.

Like many bugs, this crash was caused by unexpected interactions. While the pieces worked in isolation, subtle assumptions in their designs meant they couldn’t combine without surprising failures.

Conversation
  • In the early days of Hypothesis I actually experimented with using it to try to narrow down this sort of thing – you test for the property “when I run these tests in this order they should pass”, try to find examples of lists drawn from the set of available tests which falsify that, and then minimize the example. I keep meaning to revisit this idea as e.g. a py.test plugin.

    This would also fit in well with some of the things we talked about with working with non-deterministic test failures.

    • Scott Vokes Scott Vokes says:

      Indeed, that’s another great example for using heuristically guided randomness in testing.

      I’ve looked into running greatest tests in pseudo-random order for this reason too, but it’s tricky to do with no dynamic allocation.

  • Comments are closed.