FIX: Avoid race conditions when toggling presence state (#19648)
We need to set the local state of a channel before performing any async operations. Otherwise, multiple leave/join calls can race against each other and cause the local state to get out-of-sync with the server.
Followup to e70ed31a
This commit is contained in:
parent
083ef4c8a1
commit
25ad99637d
|
@ -67,16 +67,16 @@ class PresenceChannel extends EmberObject.extend(Evented) {
|
||||||
|
|
||||||
this.setProperties({ activeOptions });
|
this.setProperties({ activeOptions });
|
||||||
if (!this.present) {
|
if (!this.present) {
|
||||||
await this.presenceService._enter(this);
|
|
||||||
this.set("present", true);
|
this.set("present", true);
|
||||||
|
await this.presenceService._enter(this);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Mark the current user as leaving this channel
|
// Mark the current user as leaving this channel
|
||||||
async leave() {
|
async leave() {
|
||||||
if (this.present) {
|
if (this.present) {
|
||||||
await this.presenceService._leave(this);
|
|
||||||
this.set("present", false);
|
this.set("present", false);
|
||||||
|
await this.presenceService._leave(this);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -32,7 +32,7 @@ function usersFixture() {
|
||||||
|
|
||||||
acceptance("Presence - Subscribing", function (needs) {
|
acceptance("Presence - Subscribing", function (needs) {
|
||||||
needs.pretender((server, helper) => {
|
needs.pretender((server, helper) => {
|
||||||
server.get("/presence/get", (request) => {
|
server.get("/presence/get", async (request) => {
|
||||||
const channels = request.queryParams.channels;
|
const channels = request.queryParams.channels;
|
||||||
const response = {};
|
const response = {};
|
||||||
|
|
||||||
|
@ -245,10 +245,29 @@ acceptance("Presence - Subscribing", function (needs) {
|
||||||
acceptance("Presence - Entering and Leaving", function (needs) {
|
acceptance("Presence - Entering and Leaving", function (needs) {
|
||||||
needs.user();
|
needs.user();
|
||||||
|
|
||||||
|
let responseStartPromise;
|
||||||
|
let resolveResponseStartPromise;
|
||||||
|
let responseWaitPromise;
|
||||||
|
|
||||||
const requests = [];
|
const requests = [];
|
||||||
needs.hooks.afterEach(() => requests.clear());
|
|
||||||
|
needs.hooks.beforeEach(() => {
|
||||||
|
responseStartPromise = new Promise(
|
||||||
|
(resolve) => (resolveResponseStartPromise = resolve)
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
needs.hooks.afterEach(() => {
|
||||||
|
requests.clear();
|
||||||
|
responseWaitPromise = null;
|
||||||
|
responseStartPromise = null;
|
||||||
|
});
|
||||||
|
|
||||||
needs.pretender((server, helper) => {
|
needs.pretender((server, helper) => {
|
||||||
server.post("/presence/update", (request) => {
|
server.post("/presence/update", async (request) => {
|
||||||
|
resolveResponseStartPromise();
|
||||||
|
await responseWaitPromise;
|
||||||
|
|
||||||
const body = new URLSearchParams(request.requestBody);
|
const body = new URLSearchParams(request.requestBody);
|
||||||
requests.push(body);
|
requests.push(body);
|
||||||
|
|
||||||
|
@ -325,6 +344,35 @@ acceptance("Presence - Entering and Leaving", function (needs) {
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("interleaved join/leave calls work correctly", async function (assert) {
|
||||||
|
let resolveServerResponse;
|
||||||
|
responseWaitPromise = new Promise(
|
||||||
|
(resolve) => (resolveServerResponse = resolve)
|
||||||
|
);
|
||||||
|
|
||||||
|
const presenceService = this.container.lookup("service:presence");
|
||||||
|
const channel = presenceService.getChannel("/test/ch1");
|
||||||
|
|
||||||
|
const enterPromise = channel.enter();
|
||||||
|
|
||||||
|
await responseStartPromise;
|
||||||
|
|
||||||
|
const leavePromise = channel.leave();
|
||||||
|
|
||||||
|
resolveServerResponse();
|
||||||
|
|
||||||
|
await enterPromise;
|
||||||
|
await leavePromise;
|
||||||
|
|
||||||
|
assert.strictEqual(requests.length, 2, "server received two requests");
|
||||||
|
|
||||||
|
const presentChannels = requests.map((r) => r.getAll("present_channels[]"));
|
||||||
|
const leaveChannels = requests.map((r) => r.getAll("leave_channels[]"));
|
||||||
|
|
||||||
|
assert.deepEqual(presentChannels, [["/test/ch1"], []]);
|
||||||
|
assert.deepEqual(leaveChannels, [[], ["/test/ch1"]]);
|
||||||
|
});
|
||||||
|
|
||||||
test("raises an error when entering a non-existent channel", async function (assert) {
|
test("raises an error when entering a non-existent channel", async function (assert) {
|
||||||
const presenceService = this.container.lookup("service:presence");
|
const presenceService = this.container.lookup("service:presence");
|
||||||
const channel = presenceService.getChannel("/blah/does-not-exist");
|
const channel = presenceService.getChannel("/blah/does-not-exist");
|
||||||
|
|
Loading…
Reference in New Issue