Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YapDatabase cannot be created twice at same path. #538

Open
MythicLionMan opened this issue Dec 3, 2020 · 2 comments
Open

YapDatabase cannot be created twice at same path. #538

MythicLionMan opened this issue Dec 3, 2020 · 2 comments

Comments

@MythicLionMan
Copy link

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.

  • Use the workaround and create an empty file within YapDatabase init before path normalization.
  • 'Hack' the path by stripping the '/private' prefix off manually, or normalize it with a different API that doesn't consult the filesystem. (but this doesn't fix lookup issues, like a path with and without the prefix, and it isn't very future proof since it has to anticipate the behaviour of stringByStandardizingPath which may change in the future).
  • Do not normalize the path again when deregistering the database to ensure that it is consistent with the path used on launch. (but this doesn't fix lookup issues, like a path with and without the prefix).
  • Use a map table instead of a set with the YapDatabase instance as the key (or some other unique value derived from it), to ensure that the path associated with an instance is always the path that is released when it is dealloced (but this doesn't fix lookup issues, like a path with and without the prefix).
  • Don't check for duplicate paths at all since it is a bit error prone.
@deze333
Copy link
Contributor

deze333 commented Dec 11, 2020

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
Copy link

m1entus commented Apr 12, 2022

+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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants