`<PreJoin` not correctly stopping tracks after unmount

[tbscode]

Select which package(s) are affected

\@livekit/components-react

Describe the bug

When the <PreJoin component un-mounts the video light stays on and it shows that the camara is still in use.

Reproduction

This example is a minimal reproduction of the issue:

import { useState } from 'react';
import reactLogo from './assets/react.svg';
import viteLogo from '/vite.svg';
import { PreJoin } from '@livekit/components-react';
import './App.css';

function App() {
  const [count, setCount] = useState(0);
  const [prejoinVisible, setPrejoinVisible] = useState(true);

  return (
    <>
      <div>
        <button
          onClick={() => {
            setPrejoinVisible(!prejoinVisible);
          }}
        >
          {' '}
          toggle{' '}
        </button>
        {prejoinVisible && <PreJoin />}
      </div>
    </>
  );
}

export default App;

You can also find it here on Stackblitz: https://stackblitz.com/edit/vitejs-vite-bg3a4l?file=src%2FApp.jsx When you allow camera and audio permission, and then click the toggle you can see the video light stays on even though the component should have unmounted.

Logs

No response

System Info

Not applicable see stack-blitz reproduction

Severity

blocking all usage of LiveKit

Additional Information

This is kinda high prio ( only workaround we see possible atm is implementing our own pre-join )

[lukasIO]

is this actually reproducible outside of a dev build? it seems to resolve as soon as ReactStrictMode isn't enabled

[tbscode]

Indeed it does seem like it! Ok nice!

[tbscode]

Alright thanks a lot! That definitely reduces the urgency.

I'll test on our staging server and report back here.

It's kind of still a bug right? Should be working in StrictMode too and other people will probably run into the same problem :D thanks for helping!

[lukasIO]

yeah, it should definitely also work in strict mode! The weird thing is, I cannot reproduce it anymore, even in strict mode. Tried the stackblitz repo again (thanks for that btw!) and the camera indicator consistently turns off for me now even with strict mode enabled. Also tested locally in our example app with strict mode enabled and it also works there. First time I visited the stackblitz repo however I saw that it didn't turn off.

[tbscode]

I think it's maybe a concurrency issue in usePreviewTracks here: https://github.com/livekit/components-js/blob/main/packages/react/src/prefabs/PreJoin.tsx

[tbscode]

It seems to me that it appear when you give permission the first time. I.e.: clearing the permissions from the stackblitz reproduction and then reloading the page - giving the permissions again - I see the issue again.

[lukasIO]

That's what I thought as well, but even when I reset the permissions 9/10 times it works.

I think it's maybe a concurrency issue in usePreviewTracks here: https://github.com/livekit/components-js/blob/main/packages/react/src/prefabs/PreJoin.tsx

Do you see anything wrong with the logic there? We recently addressed this exact issue by refactoring the useEffect in usePreviewDevices to follow react recommendations for async requests in useEffects.

[tbscode]

Ok sadly it seems we see this issue in our app also without StrictMode. It is quite unpredictable though, seems to always work on chrome but fail about 40% of the time on firefox. So this is still 'urgent' ;)

I'm not super proficient in react async.But looking at usePreviewTracks what comes to mind is:

If needCleanup = true happens right before setTrack(localTracks) in that moment the track might not be set properly, and the cleanup will not happen. So I think this might be a race-condition, when the component unmounts already, and the setTrack(localTracks) has not run yet.

I just created this pr: https://github.com/livekit/components-js/pull/852

It does fix the issue for me, tested it by building and packaging the components locally and installing the from a file.

[lukasIO]

If needCleanup = true happens right before setTrack(localTracks) in that moment the track might not be set properly, and the cleanup will not happen. So I think this might be a race-condition, when the component unmounts already, and the setTrack(localTracks) has not run yet.

not sure I follow, all the variables are locally scoped to the function within the effect. And it's either cleanup OR setTracks

[tbscode]

Yeah I prob wasn't correct :) I've updated the PR and added some comments / a new stack-blitz with my patch.

[harisnewbie]

I'm also facing the same issue