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.

Sign up for GitHub

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

Fix network object sharing race condition within the same process #537

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Fix network object sharing race condition within the same process #537

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/shared/services/network-object.service.ts
View file
Open in desktop
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