Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create IPC locking library @matrixai/js-file-locks to introduce RWlocks in IPC #290

Open
joshuakarp opened this issue Nov 15, 2021 · 8 comments
Labels
design Requires design development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management

Comments

@joshuakarp
Copy link
Contributor

joshuakarp commented Nov 15, 2021

Specification

We'd like to investigate creating our own locking library, based off our current usage of fd-lock.

Previously we were using both proper-lockfile and fd-lock in a more conglomerated fashion. Now, we are purely using fd-lock for the Status (our refactored agent lockfile #283) and Session usage (implemented here).

Note the distinction between "locking" here. Here, we're referring to "file locking" (as per the fd prefix). For example, for the Status, we lock this status file to assert that an agent process is currently executing. Conversely, we're using async-mutex for intra-process locking (locking some resource from within a process, as opposed to between processes). Remember that we also have an existing read-write locking implementation that utilises async-mutex.

Our own library should support:

  • OS-specific read-write locks
  • Native promise API
  • Explore the addition of POSIX locks

OS-specific read-write locks should be the initial priority, and should be seen as a basic extension of functionality from fd-lock (we can use this as a source of inspiration to copy from).

Additional context

Tasks

  1. ...
  2. ...
  3. ...
@joshuakarp joshuakarp added development Standard development design Requires design labels Nov 15, 2021
@CMCDragonkai CMCDragonkai changed the title Create locking library (js-lock-fd) Create locking library js-lock-fd in order to introduce inter-process locking and rwlocks Nov 15, 2021
@CMCDragonkai
Copy link
Member

Note that async-mutex is an in-process locking, while file locking is for inter-process locking. We would want to expose rwlocks directly from flock and equivalent in windows for this to work.

@CMCDragonkai CMCDragonkai changed the title Create locking library js-lock-fd in order to introduce inter-process locking and rwlocks Create IPC locking library js-lock-fd to introduce RWlocks in IPC Nov 23, 2021
@CMCDragonkai
Copy link
Member

The src/utils.ts is now src/utils. It's now a directory with src/utils/locks.ts which contains all locking utility functions.

Here is where the RWLock based on async-mutex is. Can be used for other locking utilities that use file locking, or more abstractions are built on top of IPC file locking or in-memory async-mutex (promise-based) locking.

@CMCDragonkai
Copy link
Member

As discussed in #283 (comment), there's a problem with Status where it's possible for readStatus to get an "intermediate state" of the status file, such as between truncation and writing. This is not ideal, although a rare situation. Once we have IPC file-based RWlocking, then we can ensure that such reads are not interrupted such writes and no intermediate states are possible. Note that due to the way status works, this may require either 2 files breaking up status.json to 2 files: status.json and status.lock, or if we implement a rw-lock upgrade logic which must work on all platforms natively too.

@CMCDragonkai
Copy link
Member

Just experienced the intermittent ErrorStatusParse error that comes from not having a robust IPC. But this is temporary so for now we just have to accept that it sometimes happens.

 FAIL  tests/bin/agent/start.test.ts (71.003 s)
  ● start › start in background

    ErrorStatusParse: JSON parsing failed

      115 |       statusInfo = JSON.parse(statusData);
      116 |     } catch (e) {
    > 117 |       throw new statusErrors.ErrorStatusParse('JSON parsing failed');
          |             ^
      118 |     }
      119 |     if (!statusUtils.statusValidate(statusInfo)) {
      120 |       throw new statusErrors.ErrorStatusParse('StatusInfo validation failed', {

      at Object.readStatus (src/status/Status.ts:117:13)
      at statusInfo (src/status/Status.ts:152:16)
      at poll (src/utils/utils.ts:123:18)
      at Object.waitFor (src/status/Status.ts:150:24)
      at Object.<anonymous> (tests/bin/agent/start.test.ts:142:27)

@CMCDragonkai
Copy link
Member

I've developed a read-preferring and write-preferring in-memory RWLock for JS: https://gist.github.com/CMCDragonkai/4de5c1526fc58dac259e321db8cf5331

We may even have a general library for locking that incorporates these features.

@CMCDragonkai
Copy link
Member

Good reference for IPC locking on Linux: https://gavv.github.io/articles/file-locks/#bsd-locks-flock

MacOS and Windows might have unique things too.

@CMCDragonkai
Copy link
Member

This will be the one https://github.com/MatrixAI/js-file-locks will be imported as @matrixai/file-locks.

@CMCDragonkai CMCDragonkai changed the title Create IPC locking library js-lock-fd to introduce RWlocks in IPC Create IPC locking library @matrixai/js-file-locks to introduce RWlocks in IPC Mar 28, 2022
@CMCDragonkai
Copy link
Member

Updated this to focus on IPC locking, so @matrixai/js-async-locks can be done beforehand and there are many places where in-memory RWLock will be sufficient to solve the #294 problem. Which should be addressed in #366.

@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Requires design development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management
Development

No branches or pull requests

2 participants