Opened 3 years ago

Closed 2 years ago

Last modified 2 weeks ago

#4832 closed defect (fixed)

The API to initialize I/O resources is inherently racy

Reported by: eric@… Assignee:
Priority: Unknown Milestone:
Module: Libadblockplus Keywords:
Cc: Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29498576/
https://codereview.adblockplus.org/29498586/

Description

Semantics of the API

JsEngine uses post-construction, dynamic initialization of the file system it exposes through the _fileSystem JS object. In particular, it's possible to change the underlying file system while I/O is pending. This interface has an inherent race condition about which file system, the new or the old, a particular I/O operation is acting upon.

The problem is not confined to I/O operations considered individually. The API also permits changes of the file system between two I/O operations, because the API has no provision to ensure that the file system has not changed.

In practice, dynamic initialization provide no benefit. A client typically instantiates an engine, immediately initializes a file system for it, and then never changes thereafter.

The other kinds of I/O, web request and console, use the same kind of API, have the same problems, and are amenable to the same solutions.

Correct behavior

There are many possible correct behaviors that are free of race conditions. Here are some possibilities:

1) Remove the file system from the engine entirely. This eliminates the race condition entirely by eliminating any opportunity for it, pushing responsibility for eliminating any races onto users of JsEngine. Instead an auxiliary class would take responsibility for both a file system and its exposed JS interfaces. This removes the permanent existence of any JS file system object, which, unless statically initialized, creates the opportunity for a race condition in the first place.

If the owner of a JsEngine instance were to manage a file system and its associated JS object, it could initialize both in its constructor, providing the effect of static initialization of the file system without any explicit code dealing with life spans at all. See #4693 for a related issue, which would use this pattern for FilterEngine.

2) Initialize the file system of the engine statically at the time of construction. This removes the race condition by removing dynamic initialization.

3) Provide some mechanism for keeping dynamic initialization and providing the client the means to bind individual I/O operations to specific file systems, either explicitly or with a "current file system" context. Any mechanism like this complicates the API significantly and does not seem to provide any actual benefit.

Change History (6)

comment:1 Changed 3 years ago by eric@…

This issue is related to #1582 to provide atomic file operations, which are only well-defined with respect to a single file system.

comment:2 Changed 2 years ago by sergz

  • Review URL(s) modified (diff)

comment:3 Changed 2 years ago by sergz

  • Review URL(s) modified (diff)

comment:5 Changed 2 years ago by sergz

  • Resolution set to fixed
  • Status changed from new to closed

comment:6 Changed 7 months ago by takken3

spam

Last edited 2 weeks ago by kzar (previous) (diff)
Note: See TracTickets for help on using tickets.