Skip to content

Commit

Permalink
Fix network object sharing race condition within the same process (#537)
Browse files Browse the repository at this point in the history
  • Loading branch information
lyonsil authored Oct 6, 2023
1 parent cb98efc commit 2c3fea0
Showing 1 changed file with 13 additions and 5 deletions.
18 changes: 13 additions & 5 deletions src/shared/services/network-object.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ class MutexMap {
}
}

const mutexMap: MutexMap = new MutexMap();
const getterMutexMap: MutexMap = new MutexMap();
const setterMutexMap: MutexMap = new MutexMap();

/** This proxy enables calling functions on a network object that exists in a different process */
const createRemoteProxy = (
Expand Down Expand Up @@ -287,8 +288,8 @@ const get = async <T extends object>(
): Promise<NetworkObject<T> | undefined> => {
await initialize();

// Don't allow simultaneous gets and/or sets to run for the same network object
const lock: Mutex = mutexMap.get(id);
// Don't allow simultaneous gets to run for the same network object
const lock: Mutex = getterMutexMap.get(id);
return lock.runExclusive(async () => {
// If we already have this network object, return it
const networkObjectRegistration = networkObjectRegistrations.get(id);
Expand All @@ -299,6 +300,13 @@ const get = async <T extends object>(
const networkObjectFunctions = await getRemoteNetworkObjectFunctions(id);
if (!networkObjectFunctions) return undefined;

// Before we create a remote proxy, see if there was a race condition for a local proxy.
// It is possible we called `get`, then while awaiting the network response something else in
// this process called `set` on the object we were looking for.
const networkObjectRegistrationSecondChance = networkObjectRegistrations.get(id);
if (networkObjectRegistrationSecondChance)
return networkObjectRegistrationSecondChance.networkObject as NetworkObject<T>;

// At this point, the object exists remotely but does not yet exist locally.

// The base object created below might need a reference to the final network object. Since the
Expand Down Expand Up @@ -363,8 +371,8 @@ const set = async <T extends NetworkableObject>(
): Promise<DisposableNetworkObject<T>> => {
await initialize();

// Don't allow simultaneous gets and/or sets to run for the same network object
const lock: Mutex = mutexMap.get(id);
// Don't allow simultaneous sets to run for the same network object
const lock: Mutex = setterMutexMap.get(id);
return lock.runExclusive(async () => {
// Check to see if we already know there is a network object with this id.
if (hasKnown(id)) throw new Error(`Network object with id ${id} is already registered`);
Expand Down

0 comments on commit 2c3fea0

Please sign in to comment.