yapstudios / YapDatabase

YapDB is a collection/key/value store with a plugin architecture. It's built atop sqlite, for Swift & objective-c developers.
Other
3.35k stars 365 forks source link

YapDatabase cannot be created twice at same path. #538

Open MythicLionMan opened 3 years ago

MythicLionMan commented 3 years ago

It is an API error to create two instances of YapDatbase that reference the same database file. To enforce this YapDatabaseManager registers the normalized path of all database instances, and releases them when the instance is dealloced. When a new instance is created it checks the registry, and if another instance has registered the path then it throws an exception.

There is a bug in this procedure where a registered path can 'leak' and prevent a new instance from being allocated with the same path despite the fact that the original instance has been released.

Steps to reproduce

  1. Create a YapDatabase instance with a path that is relative to '/private'.
  2. Profit from said instance…
  3. Release the instance.
  4. Delete the database files that were created in step 1.
  5. Create a new instance with the same same path and in the same running application as step 1.

Expected behaviour

The second database instance is created and useable.

Actual behaviour

Creation of the second instance fails because the database path was not released in step 4 (the path is still visible in the 'registeredPaths' set).

Analysis

The documentation for 'stringByStandardizingPath' states that it has different behaviour for paths that have '/private' as their root based on the existence of the file reference by the path in the file system. If such a path still points to a valid file after removing the '/private' prefix, the prefix is removed (under the assumption that it is the same file). (Oddly enough the documentation for the Swift version of the method is not so detailed, even though it has the same behaviour).

Thus if Yap registers the normalized path before the database is created, when it is released and the path is normalized again it attempts to deregister a path without the '/private' prefix, while it registered one that does have the prefix, so the registration leaks. The next attempt to connect will succeed if the file remains, since the new normalized path won't have the prefix either. But if the database is deleted the new normalized path will have the prefix, and since it is still registered it will throw an exception.

Workaround

An easy workaround is to 'touch' the database file before creating a YapDatabase instance. If there is no existing database file create an empty file to ensure that normalization is consistent before and after the database are created. sqlite will happily create a database on top of an empty file.

Fix

There are a few ways to fix this.

deze333 commented 3 years ago

As a side effect of this issue, an app crash happens during YapDatabase dealloc with a stack trace like this one:

Exception Type:  EXC_CRASH (SIGABRT)
Exception Codes: 0x0000000000000000, 0x0000000000000000
Exception Note:  EXC_CORPSE_NOTIFY
Triggered by Thread:  5

Last Exception Backtrace:
0   CoreFoundation                  0x1a7b459d4 __exceptionPreprocess + 216 (NSException.m:199)
1   libobjc.A.dylib                 0x1bb4f6b54 objc_exception_throw + 56 (objc-exception.mm:565)
2   Foundation                      0x1a8cf4fd0 -[NSURL(NSURL) initFileURLWithPath:isDirectory:] + 604 (NSURL.m:0)
3   Foundation                      0x1a8cf4d68 +[NSURL(NSURL) fileURLWithPath:isDirectory:] + 44 (NSURL.m:803)
4   YapDatabase                     0x106d4d400 -[YapDatabase databaseURL_wal] + 108 (YapDatabase.m:383)
5   YapDatabase                     0x106d4e464 -[YapDatabase dealloc] + 156 (YapDatabase.m:656)
6   YapDatabase                     0x106d4de68 -[YapDatabase initWithURL:options:] + 2012 (YapDatabase.m:648)
7   MyApp                               0x106be224c @nonobjc YapDatabase.init(url:) + 32 (<compiler-generated>:0)

That's because the init exits before initializing class properties:

- (id)initWithURL:(NSURL *)inURL options:(nullable YapDatabaseOptions *)inOptions
{
    // Standardize the path.
    // This allows for fileReferenceURL's, and non-standard paths to be passed without issue.
    NSString *databasePath = [[[inURL filePathURL] path] stringByStandardizingPath];

    // Ensure there is only a single database instance per file.
    // However, clients may create as many connections as desired.
    if (![YapDatabaseManager registerDatabaseForPath:databasePath])
    {
        YDBLogError(@"Only a single database instance is allowed per file. "
                    @"For concurrency you create multiple connections from a single database instance.");
        return nil;
    }

        // ... rest of code

yet dealloc will access self.databaseURL which is NIL:

- (void)dealloc
{
    YDBLogVerbose(@"Dealloc <%@ %p: databaseName=%@>", [self class], self, [databaseURL lastPathComponent]);

    NSMutableDictionary *userInfo = [NSMutableDictionary dictionaryWithCapacity:3];
    userInfo[YapDatabaseUrlKey]    = self.databaseURL;  // in NIL state
    userInfo[YapDatabaseUrlWalKey] = self.databaseURL_wal; // CRASHES HERE
    userInfo[YapDatabaseUrlShmKey] = self.databaseURL_shm;
    // ... rest of code
m1entus commented 2 years ago

+1 Seems when want to access database from NotificationServiceExtension i have simmilar issue because my main app started + extension is running too and need access to db (and it crashing)