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.

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

Chime doesn't wait for promises creating unwanted side effects. #2572

Open
4 tasks done
andreisergiu98 opened this issue Feb 2, 2023 · 2 comments
Open
4 tasks done
Labels
feature-request New feature or request Triaged

Comments

@andreisergiu98
Copy link

andreisergiu98 commented Feb 2, 2023

What happened and what did you expect to happen?

There are some areas where chime creates promises then does not wait for them, letting them run in the background creating side effects and race conditions.

Example 1: joining and leaving a meeting

await meetingManager.join();
await meetingManager.start();
await meetingManager.leave();

If I run these instructions in order for 100 times, I expect the last meeting status to be Left, all streams to be stopped, all workers terminated. What actually happens is: chime resolves start() before the whole start process ends, so when I call leave() it is both leaving and starting at the same time. As a consequence the meeting status is either Succeeded, Left, JoinedFromAnotherDevice etc...

Workaround: add an observer before start and listen for status changes:

        const finishPromise = new Promise<MeetingStatus | undefined>((resolve) => {
            const done = (status?: MeetingStatus) => {
                clearTimeout(timeout);
                meetingManager.unsubscribeFromMeetingStatus(callback);
                resolve(status);
            };

            const timeout = setTimeout(() => {
                done();
            }, 8000);

            const callback = (status: MeetingStatus) => {
                if (status === MeetingStatus.Loading || status === MeetingStatus.Left) {
                    return;
                }
                done(status);
            };
            meetingManager.subscribeToMeetingStatus(callback);
        });

Scenario: Attendees are taking a break, the meeting is idle and as a consequence has ended. After the break is over attendees are rejoining the meeting. The meeting has ended so we are creating a new one. We are leaving the one that ended and joining the new one, but we are encountering race conditions. Another example is breakout rooms, leave room A and join room B.

Example 2: creating and destroying DefaultVideoFrameProcessorPipeline

const processor = await createBackgroundFilterProcessor(filter);
const processorPipeline = new DefaultVideoFrameProcessorPipeline(logger, [processor]);
processorPipeline.setInputMediaStream(stream);
const transformedStream = processorPipeline.getActiveOutputMediaStream();
await processorPipeline.destroy();
await processor.destroy();

Again, if I run these instructions in order for 100 times, I expect all workers to be terminated. Instead workers are hanging and there is nothing we can do about them.

Workaround: Make sure the worker is loaded before destroying the processor

async function waitForWorker(processor: VideoFrameProcessor) {
    const processorWithWorker = processor as VideoFrameProcessor & {
        worker?: Record<string, never>;
    };

    if (processorWithWorker.worker != null) {
        return;
    }

    await new Promise((resolve) => setTimeout(resolve, 100));
    return waitForWorker(processor);
}

await waitForWorker(processor);
await processor.destroy();

Have you reviewed our existing documentation?

Reproduction steps

Mentioned above

Amazon Chime SDK for JavaScript version

3.10

What browsers are you seeing the problem on?

Chromium

Browser version

109.0.5414.119

Meeting and Attendee ID Information.

No response

Browser console logs

These issues are pretty easy to debug, I can provide logs if needed.

@ltrung ltrung added the feature-request New feature or request label Feb 2, 2023
@ltrung
Copy link
Contributor

ltrung commented Feb 2, 2023

Currently we use events to let applications know when connection succeeds, stops, or has error so your application would need to listen to AudioVideoDidStart, AudioVideoDidStop events.

@andreisergiu98
Copy link
Author

andreisergiu98 commented Feb 3, 2023

I updated the workaround for VideoFrameProcessor because the previous one wasn't really working.

That being said, destroy() should make sure all pending promises and actions are cancelled or cleaned up after they finish. The same goes for leave(). We already have queues to make sure promises are executed in order, now we also have to listen for events and also watch for properties that are not even accessible. This is way harder then it should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request Triaged
Projects
None yet
Development

No branches or pull requests

3 participants