FirefoxOS/Device Storage/Worker Design

From MozillaWiki
Jump to: navigation, search

Phase 1

Expose DOMRequest and DOMCursor to workers

See bug 1167650.

DOMRequest and DOMCursor are currently only accessible on the main thread. It be supported by workers we must:

  1. Allow DOMRequest/DOMCursor to be constructed using nsIGlobalObject* (fed into DOMEventTargetHelper) in addition to nsPIDOMWindow*. The underlying Promise needs that to get a JS context and check state.
  2. DOMRequestService::FireSuccessAsync (FireSuccessAsyncTask by extension) and DOMRequestService::FireErrorAsync must be changed to dispatch to the current thread and remove the assert checks for main thread. mozilla::ThreadsafeAutoSafeJSContext should be switched for AutoJSAPI and Init(nsIGlobalObject*) (gotten from DOMRequest::GetParentObject); this will assert that the current thread is owning thread.
  3. Add Exposed=(Window,Worker) to the WebIDL definitions.

Ensure volume service is thread safe

See bug 1166320.

Volume service is designed to be accessed off the main thread, but there are problems with the current implementation.

  1. When a volume is removed via nsVolumeService::RemoveVolumeByName, the actual removal of the pointer from mVolumeArray is not protected by the array lock.
  2. When a volume is updated via nsVolumeService::UpdateVolume, clients of the service may already hold a pointer to the volume object being updated and be concurrently accessing its properties, including non-primitives such as nsString. Rather than update the object in place and risk indeterminate behaviour, the volume service should store a new volume object while holding the array lock. This way the client will have a consistent (if stale) state and may retry as appropriate if the operation fails as a result (this race condition should already be considered).

As a result, from the perspective of the volume service client, a volume object's properties are immutable.

Operations such as mount, unmount and format, may only be completed in the parent process, and thus are not a concern for web workers.

Abstract use of preferences to dedicated module

See bug 1186273.

Preferences can only be accessed on the main thread. Workers should access a cached, thread safe version of that in a dedicated module. This should merge uses of preferences throughout the code, including GlobalDirs and OverrideRootDir.

This module should be a singleton, created by nsLayoutStatics::Initialize and destroyed via ClearOnShutdown. If the singleton is missing, all device storage APIs should fail (rather than attempt to initialize given they may not be on the main thread). It should register as a listener to the preferences module to update the cache if any settings change.

Mutex protected accessors should be provided for:

  • device.storage.prompt.testing
  • device.storage.writable.name
  • device.storage.testing
  • device.storage.overrideRootDir

As well as accessors for paths on platforms other than Gonk (as exported by GlobalDirs):

  • videos
  • pictures
  • music
  • sdcard

Ideally the concept of overrideRootDir would be abstracted if possible so that nsDOMDeviceStorage can ignore it.

Type checker (DeviceStorageTypeChecker)

The string bundle service can only be accessed on the main thread and should be initialized as part of the preferences.

Observers

See bug 1186273.

The observer service can only be accessed on the main thread. Workers should access a cached, thread safe version of that in a dedicated module. This should merge uses of observers throughout the code, including FileUpdateDispatcher.

This module (ObserverManager) should be a singleton, created by nsLayoutStatics::Initialize and destroyed via ClearOnShutdown. If the singleton is missing, all device storage APIs should fail (rather than attempt to initialize given they may not be on the main thread).

When an nsDOMDeviceStorage object is created/destroyed, it should register/deregister as a listener. Any ObserverManager::Observe notifications should be proxied on the current thread to the nsDOMDeviceStorage objects, after doing any common work such as querying the volume manager.

The following events will need to be pushed to the nsDOMDeviceStorage objects on the worker threads:

  • nsPref:changed
  • volume-state-changed
  • file-watcher-update
  • disk-space-watcher

The following events may be processed inside the ObserverManager:

  • file-watcher-notify
  • download-watcher-notify

There is a special case for event listeners as presently it adds/removes an observer if a system listener is added to nsDOMDeviceStorage (see ::AddSystemEventListener and ::RemoveEventListener). Since nsDOMDeviceStorage (and going forward the ObserverManager) already listen on those events, this code should be removed as there is no need for a special case (additionally, there does not appear to be any system listeners using nsDOMDeviceStorage, so this is dead code.)

Rather than accept nsDOMDeviceStorage objects directly, it should define an interface which it implements so that other objects (i.e. DeviceStorageAreaListener) may reuse the same module to receive volume state change notifications.

Consolidate and cache use of permissions

See bug 1171170.

When checking the permissions, we must run on main thread to possibly put up allow/deny dialog and to query the parent process (if a child). To minimize delegating to the main thread from workers, we should cache the permission check result. In addition this should reduce the latency of device storage APIs due to removing an extra round trip between the parent and child processes.

In nsDOMDeviceStorage, store cached values (allow/deny) and cache state (un/initialized) for each. These are accessed on worker thread only. There are 4 access types used in conjunction with permission "device-storage:XXXtype":

  • read
  • write
  • created
  • undefined

nsDOMDeviceStorage::DeviceStorageRequest is used for this purpose presently. Other classes (i.e. DeviceStorageCursorRequest) should be changed to inherit/override methods on this instead of duplicating permission functionality. Our own versions of ::Allow and ::Cancel (e.g. ::AllowDs and ::CancelDs) should be provided and are intended to be run on the worker thread context, in addition to exposing the owning thread.

nsDOMDeviceStorage::CheckPermission should be added which accepts a DeviceStorageRequest and the permission to be verified. If the cache is initialized, it should trigger DeviceStorageRequest::AllowDs or ::CancelDs inline without posting a message. If the cache is uninitialized, it should post a runnable to the main thread to trigger nsContentPermissionUtils::AskPermission. When the permission manager's ::Allow/::Cancel is triggered, it should post a runnable to the worker thread to store the permission (i.e. nsDOMDeviceStorage::StorePermission) and then trigger DeviceStorageRequest::AllowDs or ::CancelDs as appropriate.

(Note that if it is a child process, it will most likely need to pass the request back onto the main thread in order proxy it to the parent process via PContent in the intial implementation.)

There is a special case for checking the "webapps-manage" permission. This is queried in nsDOMDeviceStorage::Init which is a synchronous function. As a result, for workers, this must be cached in the worker load info (see WorkerPrivate::GetLoadInfo) which when created is run on the main thread. A static helper function should be provided by nsDOMDeviceStorage (i.e. ::CheckAppsStoragePermission) to do the actual query, which may be reused when nsDOMDeviceStorage objects are created on the main thread. An example of something similar to this today is WorkerLoadInfo::mIndexedDBAllowed.

This is a special case for adding an event listener. While one may register without any permissions, one cannot receive the events unless one is permitted (otherwise the event is dropped). Presently we overload DOMEventTargetHelper::AddEventListener to trigger this permission check, which has several variations. We should switch to overloading DOMEventTargetHelper::EventListenerWasAdded which is meant for this purpose in derived classes and is triggered no matter how the listener is added.

Cycle collected reference counting

See bug 1171170.

nsDOMDeviceStorage, nsDOMDeviceStorageCursor, DOMRequest and DeviceStorageRequest are all cycle collected objects. The reference counting for cycle collected objects is not threadsafe (and this is unlikely to change). As such great care must be used when moving references across thread boundaries, in addition to the fact we must guarantee they are freed in the owning thread context.

The only class that can potentially change is DeviceStorageRequest. The cycle collection dependencies are from subclassing nsIContentPermissionRequest and holding DOMRequest, nsPIDOMWindow, and Blob references. The permission changes above will remove nsIContentPermissionRequest and the nsPIDOMWindow reference. The Blob reference may be converted into the threadsafe BlobImpl reference (which we already do later in the process). DOMRequest is more problematic.

Today the code carefully moves the DOMRequest from object to object with use of .forget(). If used exclusively until we post the result on the owning thread, this will prevent modifying the reference count off the owning thread. In addition to auditing the reference counting, we would also need to use NS_ProxyRelease in all destructors of the holding classes, as well as hold a reference to the owning thread, to guarantee it is freed on the correct thread if for whatever reason the DOMRequest reference was not passed off via .forget().

The alternative is centralize all references to DOM objects in a threadsafe refcounting request manager. The manager would supply the rest of the module with opaque handles identifying the DOMRequest objects. When any other object needs to interact with the DOM objects, the manager would marshal access appropriately. For posting success/error responses, it would post to the owning thread with the desired value, do any necessary JS transforms, trigger the appropriate action (FireSuccess/FireError/FireDone) and if complete, release its reference to the DOM object. The lifecycle of a single class, which is independent of the specific operations being performed, would be much easier to manage and guarantee correctness.

Device storage IPC API with child-parent processes

See bug 1171170.

PContent is the IPC object which marshalls access to the device storage IPC APIs. This is a singleton and can only be accessed on the main thread.

Sending messages happens in the following functions:

  • ContinueCursorEvent::Continue
  • nsDOMDeviceStorageCursor::Allow
  • DeviceStorageRequest::Allow
  • FileUpdateDispatcher::Observe

For FileUpdateDispatcher::Observe, this will always happen on the main thread and can be left as is. For the others, the caller may not be the main thread, in which case it must create a runnable to proxy the call to the main thread.

Receiving the responses happens in DeviceStorageRequestChild::Recv__delete__. This will need to create a runnable to proxy the response to the owning thread of the DeviceStorageRequest.

Used space cache (DeviceStorageUsedSpaceCache)

See bug 1196321.

This is only accessed on parent process, so there is no child process concerns. However since workers can also exist on the parent, it needs to be made thread safe. It may continue to be initialized on demand since it has no dependencies on the main thread. All operations besides initialization appear to run on a dedicated IO thread. As such, only the ::CreateOrGet method needs to be protected via a mutex.

MIME service

See bug 1196315.

The MIME type of a file is queried for each file when an enumeration operation is requested. The MIME service can only be accessed on the main thread; this is problematic given that this would presently require a context switch to and from each time the user calls continue on our enumeration cursor. nsExternalHelperAppService::GetTypeFromFile extracts the file extension and check then checks that against the numerous sources of MIME information (consistent with the IDL description). We should switch to extracting the file extension ourselves, use nsExternalHelperAppService:::GetTypeFromExtension to get the MIME type on the main thread for unknown extensions as is done today, and cache the result for the duration of the enumeration cursor. This balances worker performance (i.e. very likely that the number of files greatly exceeds the number of unique extensions) with accuracy (i.e. by caching only for a particular cursor request, we allow the underlying service to discover new MIME types by addition of plugins, etc if the enumeration is reattempted).

It is also queried when a file is added (prior to issuing the request) via nsDOMDeviceStorage::Add in order to determine the correct file extension. This should be moved into the create request itself, after the permission check, and before the file creation itself or sending the request to the parent process (if a child process). Note that while nsDOMDeviceStorage::Add indicates the file will be rejected by nsDOMDeviceStorage::Add(OrAppend)Named if the given extension is empty, the check is actually performed just before creating the WriteFileEvent object in DeviceStorageRequest::Allow.

Expose Device Storage to workers

See bug 1196322.

The appropriate WebIDL Exposure tags should be added to all device storage interfaces and the Navigator device storage partial interfaces.

Similar to Navigator, WorkerNavigator is exposed on worker threads. WorkerNavigator::GetDeviceStorage and ::GetDeviceStorageAreaListener must be added, along with new corresponding factory methods in device storage.

DeviceStorageAreaListener should be updated to listener on the ObserverManager, same as nsDOMDeviceStorage, in order to receive the volume state change messages.

nsDOMDeviceStorage must maintain a reference to the owning thread and ensure that all DOM interacting code runs on said thread. It must also ensure that all references to the window are replaced with either the nsIGlobalScript reference (i.e. nsDOMDeviceStorage::GetOwnerGlobal()) for DOMCursor and DOMRequest objects, nsDOMDeviceStorage for objects that accept an nsISupports reference as the parent object (i.e. Blob::Create) and nullptr for the remainder.

All existing device storage test cases should be updated/cloned to run in both a worker and the main thread contexts.

Phase 2 (to be broken down)

Device storage IPC API with child-parent processes

PContent causes device storage to depend on main thread in the child process. There are two alternatives to this:

  1. Move the device storage IPC APIs to a new IPC object which descends from PBackground.
  2. Switch to the new POSIX file IPC APIs as currently in review in bug 1141021. Also descends from PBackground.

All IPC objects can only be accessed on the thread in which they are created. When one uses PBackground, one is responsible for creating the IPC objects and thus may control what thread is used. In the singleton case, we would still need to post our IPC requests to another thread, but it would be a dedicated IO thread rather than the main thread, allowing better use of multi-core resources (in theory).

If it is possible (unconfirmed) to have an IPC object for each worker thread, then the thread context switch could be avoided.

Move file watcher notifications off main thread

The file-watcher-notify and file-watcher-update events use the observer service and thus must be sent and received on the main thread. We should investigate moving away from this to allow using other threads and avoiding the context switches. This is primarily between MTP and device storage although there may be other users of these events.

Code reorganization

  • Remove nsIDOMDeviceStorageXXX.idl files, these are not used in gecko nor gaia.
  • Modules which have an inherent main thread dependency (i.e. observers, preferences) should be moved outside nsDeviceStorage.cpp to a dedicated file.
  • There is code duplication between nsDOMDeviceStorage and DeviceStorageParent; this should be consolidated into a dedicated module/file that both depend. This equally applies to a potential switch to the POSIX IPC APIs.
  • Any code that must execute on the parent should be moved into DeviceStorageParent.
  • Any code that must execute on the child should be moved into DeviceStorageChild.
  • Accesses to parent/child paths should be abstracted from users (i.e. DeviceStorageService proxies to DeviceStorageParent/Child as appropriate) via a thin/inlined layer.
  • What remains in nsDeviceStorage.cpp should be DOM oriented and run primarily in the thread context of the object owner.