Page MenuHomePhabricator

Implement utility function in WorkerThread class to block temporarily block thread execution once all scheduled tasks complete
AbandonedPublic

Authored by marcin on Aug 22 2022, 7:04 AM.
Tags
None
Referenced Files
F3374158: D4896.diff
Tue, Nov 26, 2:10 PM
Unknown Object (File)
Sat, Nov 9, 10:30 AM
Unknown Object (File)
Sat, Nov 9, 10:19 AM
Unknown Object (File)
Sat, Nov 9, 9:42 AM
Unknown Object (File)
Sat, Nov 9, 7:40 AM
Unknown Object (File)
Fri, Nov 8, 12:45 PM
Unknown Object (File)
Oct 26 2024, 4:59 AM
Unknown Object (File)
Oct 26 2024, 4:40 AM

Details

Summary

The reason to introduce this diff is that there are react components that might try to access database while its being deleted or recreated after deletion. Such scenario has not been observed yet but it is probably because the process happens so fast so it is race condition. To avoid a crash when a component tries to access SQLite while it is being deleted we must temporarily block databaseThread argument of CommCoreModule. This differential introduces API to do so. Firstly it synchronously waits unitl all tasks in the queue complete. Then it schedules a task that block a thread execution until a promies value is set. The promise that controls how long database thread is suspended is returned to the caller.

Test Plan

The best way to test is to use this function somwhere with log statements. The order of logs should be predictable

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Add diff link to commit message

ashoat requested changes to this revision.Aug 22 2022, 7:26 PM

I'm confused what's going on here.

Firstly it synchronously waits unitl all tasks in the queue complete.

Where does this happen?

Then it schedules a task that block a thread execution until a promies value is set.

What thread's execution is blocked? How is the thread blocked, and what are the performance characteristics of the block?

It would appear to me that this needs to be a state machine with three states... 1. waiting for DB thread block to start, 2. DB thread is blocked, 3. DB thread block is concluded. Is this state represented anywhere?

native/cpp/CommonCpp/Tools/WorkerThread.cpp
33 ↗(On Diff #15821)

Can this fail? Should it be blockingWrite?

This revision now requires changes to proceed.Aug 22 2022, 7:26 PM

Firstly it synchronously waits unitl all tasks in the queue complete.

Where does this happen?

In the function we wait until taskStarted promise has its value set. (line 37) Its value is set inside the lambda that is scheduled on tasks queue. (Line 34)

Then it schedules a task that block a thread execution until a promies value is set.

What thread's execution is blocked? How is the thread blocked, and what are the performance characteristics of the block?

this->thread of WorkerThread instance is blocked. It is blocked waiting on the promise value to be set.

  1. We need to do this without any blocking calls on the main thread
  2. We should do as much of this from the database thread as possible
  3. We'll need to guarantee the order of certain operations, and that other database operations aren't going to sneak in between those operations

I found a way to atomically execute all database cleanup stages (deletion, encryption key re-creation, new database creation) on databaseThread attribute of CommCoreModule. We will continue discussion other diffs and go back to this one only if the solution I found will not be accepted.

Abandoning since an alternative solution that does not require to block main thread was created.