Make users.js robust to concurrency

backend/api.js

@@ -28,8 +28,8 @@ export class Session {
     return this.uidInfo.uid;
   }
 
-  returnUID = async () => {
+  returnUser = async () => {
-    this.uidInfo && (await this.uidInfo.returnUID());
+    this.uidInfo && (await this.uidInfo.returnUser());
   };
 
   get context() {
@@ -65,8 +65,8 @@ export class Session {
   setup = async () => {
     try {
       allSessions.add(this);
-      const { uid, returnUID } = await borrowUser(this.log);
+      const { uid, returnUser } = await borrowUser();
-      this.uidInfo = { uid, returnUID };
+      this.uidInfo = { uid, returnUser };
       this.log(`Borrowed uid ${this.uid}`);
       await this.run(this.privilegedSetup());
       if (this.config.setup) {
@@ -425,7 +425,7 @@ export class Session {
       allSessions.delete(this);
       if (this.uidInfo) {
         await this.run(this.privilegedTeardown());
-        await this.returnUID();
+        await this.returnUser();
       }
       this.ws.terminate();
     } catch (err) {

backend/sandbox.js

@@ -3,7 +3,7 @@ import { promises as fs } from "fs";
 
 import { v4 as getUUID } from "uuid";
 
-import { MIN_UID, MAX_UID, borrowUser, ignoreUsers } from "./users.js";
+import { borrowUser } from "./users.js";
 import {
   privilegedSetup,
   privilegedSpawn,
@@ -21,13 +21,8 @@ function log(msg) {
 }
 
 async function main() {
-  const dirs = await fs.readdir("/tmp/riju");
-  const uids = (
-    await Promise.all(dirs.map((dir) => fs.stat(`/tmp/riju/${dir}`)))
-  ).filter((uid) => uid >= MIN_UID && uid < MAX_UID);
-  await ignoreUsers(uids, log);
   const uuid = getUUID();
-  const { uid, returnUID } = await borrowUser(log);
+  const { uid, returnUser } = await borrowUser(log);
   await run(privilegedSetup({ uid, uuid }), log);
   const args = privilegedSpawn({ uid, uuid }, ["bash"]);
   const proc = spawn(args[0], args.slice(1), {
@@ -38,7 +33,7 @@ async function main() {
     proc.on("close", resolve);
   });
   await run(privilegedTeardown({ uid, uuid }), log);
-  await returnUID();
+  await returnUser();
 }
 
 main().catch(die);

backend/users.js

@@ -1,89 +1,104 @@
 import { spawn } from "child_process";
-import fs from "fs";
+import { promises as fs } from "fs";
 import os from "os";
 
 import AsyncLock from "async-lock";
 import _ from "lodash";
 import parsePasswd from "parse-passwd";
 
-import { privilegedUseradd, run } from "./util.js";
+import { asBool, privilegedUseradd, run, uuidRegexp } from "./util.js";
 
 // Keep in sync with system/src/riju-system-privileged.c
 export const MIN_UID = 2000;
 export const MAX_UID = 65000;
 
-const CUR_UID = os.userInfo().uid;
+function validUID(uid) {
+  return uid >= MIN_UID && uid < MAX_UID;
+}
 
-let availIds = null;
+const CUR_UID = os.userInfo().uid;
-let nextId = null;
+const ASSUME_SINGLE_PROCESS = asBool(
+  process.env.RIJU_ASSUME_SINGLE_PROCESS,
+  false
+);
+
+let initialized = false;
+let nextUserToCreate = null;
+let locallyBorrowedUsers = new Set();
+let availableUsers = new Set();
 let lock = new AsyncLock();
 
-async function readExistingUsers(log) {
+async function getCreatedUsers() {
-  availIds = parsePasswd(
+  return new Set(
-    await new Promise((resolve, reject) =>
+    parsePasswd(await fs.readFile("/etc/passwd", "utf-8"))
-      fs.readFile("/etc/passwd", "utf-8", (err, data) => {
+      .map(({ uid }) => parseInt(uid))
-        if (err) {
+      .filter((uid) => !isNaN(uid) && validUID(uid))
-          reject(err);
+  );
-        } else {
+}
-          resolve(data);
+
-        }
+async function getActiveUsers() {
-      })
+  return new Set(
+    (
+      await Promise.all(
+        (await fs.readdir("/tmp/riju"))
+          .filter((name) => name.match(uuidRegexp))
+          .map((name) => fs.stat(`/tmp/riju/${name}`))
+      )
     )
-  )
+      .map(({ uid }) => uid)
-    .filter(({ username }) => username.startsWith("riju"))
+      .filter(validUID)
-    .map(({ uid }) => parseInt(uid))
+  );
-    .filter((uid) => !isNaN(uid) && uid >= MIN_UID && uid < MAX_UID)
-    .reverse();
-  nextId = (_.max(availIds) || MIN_UID - 1) + 1;
-  log(`Found ${availIds.length} existing users, next is riju${nextId}`);
 }
 
 async function createUser(log) {
-  if (nextId >= MAX_UID) {
+  if (nextUserToCreate >= MAX_UID) {
     throw new Error("too many users");
   }
-  const uid = nextId;
+  const uid = nextUserToCreate;
   await run(privilegedUseradd(uid), log);
-  log(`Created new user with ID ${uid}`);
+  nextUserToCreate += 1;
-  nextId += 1;
   return uid;
 }
 
-export async function ignoreUsers(uids, log) {
-  await lock.acquire("key", async () => {
-    if (availIds === null || nextId === null) {
-      await readExistingUsers(log);
-    }
-    const uidSet = new Set(uids);
-    if (uidSet.size > 0) {
-      const plural = uidSet.size !== 1 ? "s" : "";
-      log(
-        `Ignoring user${plural} from open session${plural}: ${Array.from(uidSet)
-          .sort()
-          .map((uid) => `riju${uid}`)
-          .join(", ")}`
-      );
-    }
-    availIds = availIds.filter((uid) => !uidSet.has(uid));
-  });
-}
-
 export async function borrowUser(log) {
   return await lock.acquire("key", async () => {
-    if (availIds === null || nextId === null) {
+    if (!initialized || !ASSUME_SINGLE_PROCESS) {
-      await readExistingUsers(log);
+      const createdUsers = await getCreatedUsers();
+      const activeUsers = await getActiveUsers();
+      if (createdUsers.size > 0) {
+        nextUserToCreate = _.max([...createdUsers]) + 1;
+      } else {
+        nextUserToCreate = MIN_UID;
+      }
+      // If there are new users created, we want to make them
+      // available (unless they are already active). Similarly, if
+      // there are users that have become inactive, we want to make
+      // them available (unless they are already borrowed locally).
+      for (const user of createdUsers) {
+        if (!activeUsers.has(user) && !locallyBorrowedUsers.has(user)) {
+          availableUsers.add(user);
+        }
+      }
+      // If there are users that have become active, we want to make
+      // them unavailable.
+      for (const user of activeUsers) {
+        availableUsers.delete(user);
+      }
+      initialized = true;
     }
-    let uid;
+    if (availableUsers.size === 0) {
-    if (availIds.length > 0) {
+      availableUsers.add(await createUser(log));
-      uid = availIds.pop();
-    } else {
-      uid = await createUser(log);
     }
+    // https://stackoverflow.com/a/32539929/3538165
+    const user = availableUsers.values().next().value;
+    locallyBorrowedUsers.add(user);
+    availableUsers.delete(user);
     return {
-      uid,
+      uid: user,
-      returnUID: async () => {
+      returnUser: async () => {
         await lock.acquire("key", () => {
-          availIds.push(uid);
+          locallyBorrowedUsers.delete(user);
+          availableUsers.add(user);
         });
       },
     };

backend/util.js

@@ -129,3 +129,23 @@ export const log = {
   warn: console.error,
   error: console.error,
 };
+
+// https://gist.github.com/bugventure/f71337e3927c34132b9a
+export const uuidRegexp = /^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$/;
+
+export function asBool(value, def) {
+  if (def === undefined) {
+    throw new Error("asBool needs an explicit default value");
+  }
+  if (!value) {
+    return def;
+  }
+  value = value.toLowerCase().trim();
+  if (["y", "yes", "1", "on"].includes(value)) {
+    return true;
+  }
+  if (["n", "no", "0", "off"].includes(value)) {
+    return false;
+  }
+  throw new Error(`asBool doesn't understand value: ${value}`);
+}

docker/app/Dockerfile

@@ -11,3 +11,4 @@ RUN chown root:riju system/out/*-privileged && chmod a=,g=rx,u=rwxs system/out/*
 
 USER riju
 CMD ["make", "server"]
+ENV RIJU_ASSUME_SINGLE_PROCESS 1