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

Fix E2EE Session Initialization Race Condition #2635

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shwetd19
Copy link

Problem

When enabling E2EE, session initialization can be triggered twice through different paths:

  1. Via olmAdapter.initSessions() from setEnabled
  2. Via onParticipantPropertyChanged from setLocalParticipantProperty

This causes runtime errors when the second initialization attempt finds an existing session.

Solution

I've created a new E2EESessionManager class that keeps track of all session states in one place. This prevents multiple initialization attempts and handles them more gracefully. When a second initialization is attempted, instead of failing, it either returns the existing session or waits for the ongoing initialization to complete.

Implementation Details

  • Uses Map to track session states and initialization promises
  • Implements state machine for session lifecycle management
  • Provides safe access methods with proper error boundaries
  • Integrates with existing OlmAdapter without breaking changes

Related Issues

Fixes #2587 - Runtime error when E2EE is enabled

Implemented E2EESessionManager to handle race conditions during OLM session initialization. This fixes runtime errors that occur when sessions are initialized multiple times due to parallel execution paths through setEnabled and participant property changes.
@shwetd19
Copy link
Author

Hey @saghul can you please review my PR as per fix of #2587

@jitsi-jenkins
Copy link

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

@shwetd19
Copy link
Author

Hey @jitsi-jenkins , I've signed the CLA

@saghul
Copy link
Member

saghul commented Feb 10, 2025

Seems to be going in the right direction. I suppose you plan on integrating this into the existing flow?

@shwetd19
Copy link
Author

shwetd19 commented Feb 10, 2025

Yes @saghul !

@shwetd19
Copy link
Author

Hey @saghul @jitsi-jenkins can you please suggest any changes if required ?

@saghul
Copy link
Member

saghul commented Feb 11, 2025

See ths CI failures in GH actions. Also the change is not complete / functional. Nobody is importing or calling the new classes you defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runtime error when E2EE is enabled
3 participants