Toast: Race condition on remove

[zavog]

Describe the bug

When removing a toast and adding one or more toasts at the same time, the wrong toast may be removed.

The following is the current code in Toast.vue. When a toast is added after index = i and before splice, then the wrong toast is removed, which unfortunately might not even match the condition that was passed to remove.

remove(params) {
    let index = -1;

    for (let i = 0; i < this.messages.length; i++) {
        if (this.messages[i] === params.message) {
            index = i;
            break;
        }
    }

    this.messages.splice(index, 1);
    this.$emit(params.type, { message: params.message });
}

Reproducer

https://stackblitz.com/edit/primevue-create-vue-typescript-issue-template-gkirw2?file=src%2FApp.vue

PrimeVue version

3.48.0

Vue version

3.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

No response

Steps to reproduce the behavior

Click the "Trigger" button in the Stackblitz

Expected behavior

The remove function should never remove a task that doesn't match the condition

[ZiadJ]

Yup, it only removes the last toast added regardless of the identifier you provide.

[mertsincan]

Thanks a lot for your report! I set a milestone for it. We'll check it before the milestone is released.

[marknn3]

This is not a race condition.

Root Cause in Toast.vue

• The remove method tries to find message in the messages array with object equality.
  (if (this.messages[i] === params.message))
• But all stored message objects are converted into proxy objects when a new toast is added, because this.messages is reactive.
  (this.messages = [...this.messages, message])
• This causes the object equality condition to fail.
• In this case the found index is -1 and splice will remove the last item. (this.messages.splice(index, 1))

Possible Fix

Use message.id for the equality test because the stored objects are not guaranteed to be the same instance:
if (this.messages[i].id === params.message.id)

This will result in the intended behaviour (as if it was object equality) because the id's are unique.

Temporary Workaround

I created a wrapper around useToast which always converts the passed message object into a reactive proxy object.
With this workaround the correct toast is removed.

[dragomirweb]

This is not a race condition.

Root Cause in Toast.vue

• The remove method tries to find message in the messages array with object equality.
  (if (this.messages[i] === params.message))
• But all stored message objects are converted into proxy objects when a new toast is added, because this.messages is reactive.
  (this.messages = [...this.messages, message])
• This causes the object equality condition to fail.
• In this case the found index is -1 and splice will remove the last item. (this.messages.splice(index, 1))

Possible Fix

Use message.id for the equality test because the stored objects are not guaranteed to be the same instance: if (this.messages[i].id === params.message.id)

This will result in the intended behaviour (as if it was object equality) because the id's are unique.

Temporary Workaround

I created a wrapper around useToast which always converts the passed message object into a reactive proxy object. With this workaround the correct toast is removed.

In my project I ended up patching with this updated remove function:
`remove(params) {
const index = this.messages.findIndex((m) => m.id === params.message.id);

        if (index !== -1) {
            this.messages.splice(index, 1);
            this.$emit(params.type, { message: params.message });
        }
    },

`