Skip to content

Commit 8aa3565

Browse files
committed
fix(resource-rental-pool): fix race condition caused some rentals to not be added to the pool
1 parent 69a5c3a commit 8aa3565

File tree

1 file changed

+40
-16
lines changed

1 file changed

+40
-16
lines changed

src/resource-rental/resource-rental-pool.ts

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -229,22 +229,46 @@ export class ResourceRentalPool {
229229
createAbortSignalFromTimeout(signalOrTimeout),
230230
this.abortController.signal,
231231
);
232-
return Promise.any([
233-
this.createNewResourceRental(signal),
234-
this.acquireQueue.get(signal).then((rental) => {
235-
this.logger.info("A rental became available in the pool, using it instead of creating a new one");
236-
return rental;
237-
}),
238-
])
239-
.catch((err: AggregateError) => {
240-
// if all promises fail (i.e. the signal is aborted by the user) then
241-
// rethrow the error produced by `createNewResourceRental` because it's more relevant
242-
throw err.errors[0];
243-
})
244-
.finally(() => {
245-
ac.abort();
246-
cleanup();
247-
});
232+
233+
type RaceResult = { source: "create" | "queue"; rental: ResourceRental };
234+
235+
const createPromise = this.createNewResourceRental(signal);
236+
const queuePromise = this.acquireQueue.get(signal);
237+
238+
const wrappedCreate = createPromise.then((rental): RaceResult => ({ source: "create", rental }));
239+
const wrappedQueue = queuePromise.then((rental): RaceResult => {
240+
this.logger.info("A rental became available in the pool, using it instead of creating a new one");
241+
return { source: "queue", rental };
242+
});
243+
244+
try {
245+
const winner = await Promise.any([wrappedCreate, wrappedQueue]);
246+
247+
if (winner.source === "queue") {
248+
// The rental from the queue won, but it's possible that an agreement is
249+
// currently being signed in the `createPromise`. If that completes, we should
250+
// salvage the rental instead of letting it go to waste.
251+
createPromise
252+
.then((rental) => {
253+
this.logger.info("Agreement was signed but not used, adding it back to the pool", {
254+
agreementId: rental.agreement.id,
255+
});
256+
this.passResourceRentalToWaitingAcquireOrBackToPool(rental);
257+
})
258+
.catch(() => {
259+
// Creation was aborted or failed, nothing to salvage
260+
});
261+
}
262+
263+
return winner.rental;
264+
} catch (err) {
265+
// If all promises fail (i.e. the signal is aborted by the user) then
266+
// rethrow the error produced by `createNewResourceRental` because it's more relevant
267+
throw (err as AggregateError).errors[0];
268+
} finally {
269+
ac.abort();
270+
cleanup();
271+
}
248272
}
249273

250274
/**

0 commit comments

Comments
 (0)