1736844 - Allow sqlite3_interrupt call on read/write storage connection

Status: RESOLVED FIXED

(Core :: SQLite and Embedded Database Bindings, task)

Description

[Haiyang Xu]

As [:mak] commented in Bug1588068, modify the Connection::Interrupt() interface to support interrupt on read/write connection safely.

Comment 1

[Marco Bonardo [:mak]]

Jan suggested adding a propertybag to synchronous openDatabase, that is probably a good first step towards the right direction.

Then we could add an "interruptible" flag, if set we'd just annotate that the connection can be interrupted, and let interrupt() go through, it would be up to consumers to ensure they don't misuse it. We should probably add comments in the idl to ensure consumers of interrupt understand they must take additional care on shutdown. We should probably also check interrupt is not invoked on the same thread used by the connection, because it would be pointless. And we may want to add some kind of thread synchronization, the connection principal thread (owning thread for sync connection, helper thread for pure-async) should likely wait while interrupt is running, the risk is interrupt may end up interrupting work that was added after the fact.

Long term, we could remove this interruptible flag, and rather detect if the connection is created off the main-thread, if that case we should disallow any usage of the async APIs and the helper thread, and allow interrupt (single threaded connection). That way we'd end up with 3 types of connections:

1. main-thread with helper thread (deprecated, no new consumers should be added, existing consumers should be updated when possible)
2. synchronous off main-thread
3. asynchronous on main-thread

Comment 2

[Haiyang Xu]

Attachment: Bug 1736844 - Add interrupt flag to storage service methods opening database; r=#dom-storage-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ef0b43b28f1
Add interrupt flag to storage service methods opening database; r=dom-storage-reviewers,mak,janv

Comment 4

[Marian-Vasile Laza]

Backed out changeset 0ef0b43b28f1 (Bug 1736844) for causing gtest failures.
Backout link
Push with failures
Failure Log

Comment 5

[Jari Jalkanen]

The failures were death tests and the crashes were expected but they have now been removed.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/434682c0ba50
Add interrupt flag to storage service methods opening database; r=dom-storage-reviewers,mak,janv

Comment 7

[Jari Jalkanen]

Attachment: Bug 1736844 - Add unit test for synchronous connection interrupt. r=#dom-storage-reviewers

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c294763bf35
Add unit test for synchronous connection interrupt. r=dom-storage-reviewers,janv

Comment 9

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/434682c0ba50
https://hg.mozilla.org/mozilla-central/rev/0c294763bf35