1270488 - The application crashes when clearing browsing data while schedule syncing

Status: NEW

(Firefox for iOS :: Data Storage, defect)

Description

[Simon Basca [:SimonB]]

Attachment: Crash log

Build: 4.0.0b7
iOS: 8.4

Steps to reproduce:
1. Sign in to a Firefox account with browsing data
2. Quickly enter Clear Private Data menu and Clear Private Data
3. Uncheck Clear Cache button
4. Clear Private Data
5. Check Clear Cache button
6. Clear Private Data

Actuatl results:
- The application will crash

Note:
- This issue also occurs if the browsing data will be cleared at the same time as a schedule sync will take place.

Comment 1

[Aaron Train [:aaronmt]]

Thread 6 name:  Dispatch queue: SQLite connection: /private/var/mobile/Containers/Shared/AppGroup/89FF74FA-42A7-4213-9223-84AE1F052D22/profile.profile/browser.
Thread 6:
0   Storage                       	0x00000001019611c0 sqlite3GetVarint32 (sqlite3.c:27700)
1   Storage                       	0x00000001019610d0 btreeParseCellPtr (sqlite3.c:56597)
2   Storage                       	0x0000000101970b74 sqlite3BtreeDataSize (sqlite3.c:59631)
3   Storage                       	0x000000010196b758 sqlite3VdbeExec (sqlite3.c:75455)
4   Storage                       	0x000000010194bbb4 sqlite3_step (sqlite3.c:71811)
5   Storage                       	0x000000010192f1d0 ConcreteSQLiteDBConnection.executeChange(String, withArgs : [AnyObject?]?) -> NSError? (SwiftData.swift:624)
6   Storage                       	0x00000001019301dc protocol witness for SQLiteDBConnection.executeChange(String, withArgs : [AnyObject?]?) -> NSError? in conformance ConcreteSQLiteDBConnection (SwiftData.swift:600)
7   Storage                       	0x000000010181bb40 SQLiteRemoteClientsAndTabs.(insertOrUpdateTabsForClientGUID(String?, tabs : [RemoteTab]) -> Deferred<Maybe<Int>>).(closure #1) (SQLiteRemoteClientsAndTabs.swift:76)
8   Storage                       	0x0000000101815640 partial apply for SQLiteRemoteClientsAndTabs.(insertOrUpdateTabsForClientGUID(String?, tabs : [RemoteTab]) -> Deferred<Maybe<Int>>).(closure #1) (SQLiteRemoteClientsAndTabs.swift:0)
9   Storage                       	0x00000001019036e8 BrowserDB.(transaction(synchronous : Bool, err : inout NSError?, callback : (connection : SQLiteDBConnection, err : inout NSError?) -> Bool) -> NSError?).(closure #1) (BrowserDB.swift:274)
10  Storage                       	0x000000010193618c SwiftData.(transaction(synchronous : Bool, transactionClosure : (db : SQLiteDBConnection) -> Bool) -> NSError?).(closure #1) (SwiftData.swift:157)
11  Storage                       	0x0000000101935f10 SwiftData.(withConnection(SwiftData.Flags, synchronous : Bool, cb : (db : SQLiteDBConnection) -> NSError?) -> NSError?).(closure #3) (SwiftData.swift:131)
12  Storage                       	0x000000010191fedc thunk (SQLiteBookmarksBase.swift:0)
13  libdispatch.dylib             	0x0000000198bb5950 _dispatch_client_callout + 12
14  libdispatch.dylib             	0x0000000198bbf1e0 _dispatch_barrier_sync_f_invoke + 72
15  Storage                       	0x0000000101923f18 SwiftData.withConnection(SwiftData.Flags, synchronous : Bool, cb : (db : SQLiteDBConnection) -> NSError?) -> NSError? (SwiftData.swift:132)
16  Storage                       	0x0000000101924624 SwiftData.transaction(synchronous : Bool, transactionClosure : (db : SQLiteDBConnection) -> Bool) -> NSError? (SwiftData.swift:172)
17  Storage                       	0x00000001018fc4bc BrowserDB.transaction(synchronous : Bool, err : inout NSError?, callback : (connection : SQLiteDBConnection, err : inout NSError?) -> Bool) -> NSError? (BrowserDB.swift:275)
18  Storage                       	0x00000001018fc3fc BrowserDB.transaction(inout NSError?, callback : (connection : SQLiteDBConnection, err : inout NSError?) -> Bool) -> NSError? (BrowserDB.swift:268)
19  Storage                       	0x0000000101815440 SQLiteRemoteClientsAndTabs.insertOrUpdateTabsForClientGUID(String?, tabs : [RemoteTab]) -> Deferred<Maybe<Int>> (SQLiteRemoteClientsAndTabs.swift:102)
20  Storage                       	0x0000000101815078 SQLiteRemoteClientsAndTabs.insertOrUpdateTabs([RemoteTab]) -> Deferred<Maybe<Int>> (SQLiteRemoteClientsAndTabs.swift:63)
21  Storage                       	0x000000010181a3f0 protocol witness for RemoteClientsAndTabs.insertOrUpdateTabs([RemoteTab]) -> Deferred<Maybe<Int>> in conformance SQLiteRemoteClientsAndTabs (SQLiteRemoteClientsAndTabs.swift:62)
22  Client                        	0x00000001002d85a8 BrowserProfile.storeTabs([RemoteTab]) -> Deferred<Maybe<Int>> (Profile.swift:397)
23  Client                        	0x0000000100258604 AppDelegate.(tabManagerWillStoreTabs([Tab]) -> ()).(closure #1) (AppDelegate.swift:535)
24  Client                        	0x00000001000f8b68 thunk (PasscodeViews.swift:0)
25  libdispatch.dylib             	0x0000000198bb5990 _dispatch_call_block_and_release + 20
26  libdispatch.dylib             	0x0000000198bb5950 _dispatch_client_callout + 12
27  libdispatch.dylib             	0x0000000198bbf6ec _dispatch_after_timer_callback + 88
28  libdispatch.dylib             	0x0000000198bb5950 _dispatch_client_callout + 12
29  libdispatch.dylib             	0x0000000198bcc00c _dispatch_source_latch_and_call + 2048
30  libdispatch.dylib             	0x0000000198bb7ab8 _dispatch_source_invoke + 284
31  libdispatch.dylib             	0x0000000198bc2314 _dispatch_root_queue_drain + 716
32  libdispatch.dylib             	0x0000000198bc3c48 _dispatch_worker_thread3 + 104
33  libsystem_pthread.dylib       	0x0000000198d95228 _pthread_wqthread + 812
34  libsystem_pthread.dylib       	0x0000000198d94eec start_wqthread + 0

Comment 2

[James Hugman [:jhugman] [@jhugman]]

This is ostensibly because SQLite does not support writing to the same db from two different threads (it supports multithreaded reads).

This is a relatively simple fix in the narrow sense: expose the syncQueue to allow clearing jobs to be run on it.

This _does_ leave open the possibility of pressing clear private data and it not immediately happening, and not providing any feedback to the user that this is the case. 

The better fix would be to pause/cancel an ongoing sync which is probably beyond the scope of this release.

Attaching a PR just in case.

Comment 3

[James Hugman [:jhugman] [@jhugman]]

Attachment: Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1847

Comment 4

[Richard Newman [:rnewman]]

(In reply to James Hugman [:jhugman] [@jhugman] from comment #2)
> This is ostensibly because SQLite does not support writing to the same db
> from two different threads (it supports multithreaded reads).

But SwiftData serializes all DB operations onto a single queue. So long as we're using the same reference to the DB handle -- i.e., the same SwiftData instance, probably inside the same BrowserDB instance -- then there is no DB concurrency.

If we're not using the same SwiftData instance… that's a/the bug.

I do agree that having CPD happen strictly before or strictly after any current sync, by running on the same task queue, is a good solution. But if that happens and CPD is still opening a second DB handle, then that's wrong.

Comment 5

[Stephan Leroux [:sleroux]]

Comment on attachment 8755499 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1847

Richard gave much better feedback than I would have provided above. I'm going to clear myself from review since this got bumped down to tracking+, unless there is something else you want me to check out!