1803062 - FS: SyncAccessHandle should do all IO off the worker thread

Status: RESOLVED FIXED

(Core :: DOM: File, task, P2)

Description

[Jan Varga [:janv]]

Bug 1789590 is going to use a sync IPC message for proxying QuotaObject::MaybeUpdateSize to the parent process (the sync IPC will eventually get eliminated in future). Doing sync IPC on the main thread or on the PBackground thread or a DOM worker thread is not desired so we need to move all IO off the worked thread. We already do reads and writes off the worker thread, but there are other methods which do IO directly on the worker thread, including the close operation.

The close operation is a bit tricky, because if we move the IO for it off the worker thread, close() can't be sync anymore and we need to prevent worker shutdown until close is actually finished using StrongWorkerRef.

Comment 1

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Switch from ordinary bool errors to nsresult errors in code for handling sync loops; r=#dom-workers

Comment 2

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Add a fallible way to create FileSystemSyncAccessHandle objects; r=#dom-storage

Depends on D163559

Comment 3

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Prevent worker shutdown until FileSystemSyncAccessHandles are closed; r=#dom-storage

Depends on D163562

Comment 4

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Don't call FileSystemSyncAccessHandle::Close when the object is already closed; r=#dom-storage

Depends on D163563

Comment 5

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Prepare a way for using other FileSystemSyncAccessHandle states besides the open/closed state; r=#dom-storage

Depends on D163564

Comment 6

[Jan Varga [:janv]]

Attachment: Bug 1803062 - GetCurrentSerialEventTarget should return the nested event target when a sync loop is running; r=#dom-workers

Depends on D163559

Comment 7

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Change FileSystemSyncAccessHandle::Close to be async; r=#dom-storage

Depends on D163565

Comment 8

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Create a task queue on top of the STS for each FileSystemSyncAccessHandle; r=#dom-storage

Depends on D163842

Comment 9

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Add a helper class for automatic proxy releasing of objects; r=#dom-storage

Depends on D163937

Comment 10

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Read, write and close streams on the task queue; r=#dom-storage

Depends on D163938

Comment 11

[Jan Varga [:janv]]

Attachment: WIP: Bug 1803062 - Remove obsolete comment; r=#dom-storage

Depends on D163939

Comment 12

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Truncate, GetSize and Flush on the task queue; r=#dom-storage

Depends on D163940

Comment 13

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Change the complete callback of AsyncCopy to MoveOnlyFunction; r=#dom-storage

This is needed to prevent build errors when AsyncCopy is called inside of a
lambda function.

Depends on D163941

Comment 14

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Use mStream in ReadOrWrite only on the task queue; r=#dom-storage

Depends on D163942

Comment 15

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Deserialize streams only on the task queue; r=#dom-storage

Depends on D163943

Comment 16

[Jan Varga [:janv]]

Attachment: Bug 1803062 - Expose the sync close operation using a dedicated method; r=#dom-storage

Depends on D163563

Comment 17

[Jan Varga [:janv]]

https://treeherder.mozilla.org/jobs?repo=try&revision=378b35340375283283791501ade4be892add1006

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c8c20bcccd47
Switch from ordinary bool errors to nsresult errors in code for handling sync loops; r=dom-worker-reviewers,smaug

Comment 19

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/c8c20bcccd47

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6499b9cc1cfc
GetSerialEventTarget should return the nested event target when a sync loop is running; r=asuth

Comment 21

[Serban Stanca [:SerbanS]]

Backed out for causing worker related failures.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-TIMEOUT | /xhr/xmlhttprequest-timeout-worker-synconworker.html?timeout hit before load | expected OK

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3b1d6fc7b15
GetCurrentSerialEventTarget should return the nested event target when a sync loop is running; r=asuth

Comment 23

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/d3b1d6fc7b15

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1486ec3ed952
Add a fallible way to create FileSystemSyncAccessHandle objects; r=dom-storage-reviewers,jari
https://hg.mozilla.org/integration/autoland/rev/e34378476209
Prevent worker shutdown until FileSystemSyncAccessHandles are closed; r=dom-storage-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/a784ec790ea5
Expose the sync close operation using a dedicated method; r=smaug
https://hg.mozilla.org/integration/autoland/rev/0ba70d490377
Don't call FileSystemSyncAccessHandle::Close when the object is already closed; r=dom-storage-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/da468b3c44d0
Prepare a way for using other FileSystemSyncAccessHandle states besides the open/closed state; r=dom-storage-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/4cec2499b6e9
Change FileSystemSyncAccessHandle::Close to be async; r=dom-storage-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/e7809a501f21
Create a task queue on top of the STS for each FileSystemSyncAccessHandle; r=dom-storage-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/08f90593777f
Add a helper class for automatic proxy releasing of objects; r=dom-storage-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/8dbe876aeb90
Read, write and close streams on the task queue; r=dom-storage-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/b0354e926ca9
Truncate, GetSize and Flush on the task queue; r=dom-storage-reviewers,jari
https://hg.mozilla.org/integration/autoland/rev/2165493fffb7
Change the complete callback of AsyncCopy to MoveOnlyFunction; r=dom-storage-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/29757b4ea091
Use mStream in ReadOrWrite only on the task queue; r=dom-storage-reviewers,jari
https://hg.mozilla.org/integration/autoland/rev/020dd83e31e9
Deserialize streams only on the task queue; r=dom-storage-reviewers,jesup

Comment 25

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/1486ec3ed952
https://hg.mozilla.org/mozilla-central/rev/e34378476209
https://hg.mozilla.org/mozilla-central/rev/a784ec790ea5
https://hg.mozilla.org/mozilla-central/rev/0ba70d490377
https://hg.mozilla.org/mozilla-central/rev/da468b3c44d0
https://hg.mozilla.org/mozilla-central/rev/4cec2499b6e9
https://hg.mozilla.org/mozilla-central/rev/e7809a501f21
https://hg.mozilla.org/mozilla-central/rev/08f90593777f
https://hg.mozilla.org/mozilla-central/rev/8dbe876aeb90
https://hg.mozilla.org/mozilla-central/rev/b0354e926ca9
https://hg.mozilla.org/mozilla-central/rev/2165493fffb7
https://hg.mozilla.org/mozilla-central/rev/29757b4ea091
https://hg.mozilla.org/mozilla-central/rev/020dd83e31e9