Properly cleaning up [avoiding EXC_BAD_ACCESS on FCModelDatabaseQueueOperation]

[johncblandii]

Our app has sensitive data so we have an inactivity timeout. When the timeout occurs they have to sign in again and we close the database. Upon signing in again, we open the database and :boom:!

EXC_BAD_ACCESS on FCModelDatabaseQueueOperation.

0x2c542d4: jmp 0x2c5426a ; object_cxxDestructFromClass(objc_object*, objc_class*) + 22

What is the proper way of cleaning up/closing the db?

Creating db:

- (void)open:(NSString *)databasePath{
    [self.logger debug:@"Opening database: %@", databasePath];

    [FCModel openDatabaseAtPath:databasePath withSchemaBuilder:^(FMDatabase *db, int *schemaVersion) {
        self.database = db;

        self.database.logsErrors = YES;
        self.database.crashOnErrors = NO;
        self.database.traceExecution = NO;

        [self.database beginTransaction];

        self.schemaVersion = *schemaVersion;

        [self runMigrations];

        *schemaVersion = self.schemaVersion;

        [self.database commit];

        [self.logger debug:@"Database ready: %@", databasePath];
    }];
}

...

- (void)close{
    if (self.database) {
        [self.database close];
    }
}

On timeout: [DBClass close];

Thoughts? What's the best way to address this?

[marcoarment]

I never considered the case of callers saving the FMDatabase instance for later use. It's a bad idea — FCModel should be solely responsible for managing that object.

Do you need it only for closing, or other purposes? If only closing, you can use the newly public closeDatabase function. The only downsides are, currently:

• There's no official way to tell whether FCModel's database is currently closed
• There's no checks in the other FCModel functions to see if the database is closed and fail somehow

I'm interested in fixing both. The first is obvious, but in the case of the second, what should happen if you call, say, instanceWithPrimaryKey: or save on an FCModel when the database is closed?

Should that raise an exception, which could cause crashes if you retain FCModel instances after closeDatabase (which you really shouldn't, and it even NSLogs a warning for each one if you do), or should those calls safely (and quietly?) return nil/0/OK results, much like when you accidentally send messages to nil?

[johncblandii]

Yeah, so the errors are definitely a problem and I solved it by simply overriding the error methods; which I didn't like doing.

Ahh...I didn't know about the closeDatabase method. I'll definitely replace my implementation to use that.

My first inclination is to say raise a specific exception on save so we can trap it and break a loop [vs continuing to try over and over again in the loop]. For retrieving, I would expect a nil.

I probably do have some instances lingering after close. I'll dig into that and make sure I clear my instances. Is it enough to null my array or do I need to call an FCModel method?

I know that's up to use to not call it before opened or after close but with background sync sometimes things are mid-flight. Whatever FCModel can do to help in this scenario would be helpful for sure.

[marcoarment]

Here's how I deal with background sync/downloading with closeDatabase. Obviously, it's implementation-specific, but you get the idea.

// NSOperationQueue
[self.syncQueue cancelAllOperations];

// immediately cancels all NSURLSession downloads it knows about, and
//  any others that call delegate methods after that will be canceled
[self.downloadQueue prepareForLogOut];
self.downloadQueue = nil;

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
    if (! self.syncQueue.isSuspended) {
        [self.syncQueue waitUntilAllOperationsAreFinished];
    }
    self.syncQueue = nil;

    [FCModel closeDatabase];

    // then dispatch to main thread to show the "logged out" state
});

Try calling closeDatabase in development and it'll console-log a list of every still-active FCModel instance. Very useful for this.

Meanwhile, let me work on those error checks.

[johncblandii]

I've been looking to add a queue. I guess I need to stop putting it off and get on it. Thx for the snippet.

I'll report back tomorrow when I get a chance to implement the close.

[johncblandii]

Yep...closeDatabase spits out quite a few "Don't do this" statements. :-D #cleanupmode

[johncblandii]

Commit looks good. I'll test it out at work this week.