Skip to content

Latest commit

 

History

History
220 lines (166 loc) · 7.42 KB

File metadata and controls

220 lines (166 loc) · 7.42 KB

Lecture System Collision Analysis

Critical Issue: Multiple Clients Overriding Shared Context

Executive Summary

The current implementation of the /startLecture API in the RAG Backend Lucy system has a critical collision issue where multiple clients starting the same lecture will share and potentially override each other's context, leading to data corruption and session conflicts.


The Collision Problem

Root Cause

Looking at line 81 in app/websockets/lecture.py:

lecture_states[lecture_id] = {}

This line will overwrite ALL existing data for that lecture_id, including data from other clients/robots that may have already started the same lecture.

How the Collision Happens

Scenario 1: Sequential Client Starts

  1. Client A starts lecture with lecture_id = "abc123" and connectrobot = "robot1"

    • Creates: lecture_states["abc123"]["robot1"] with session data
    • Sets global contents and time_list for this lecture
  2. Client B starts the SAME lecture with lecture_id = "abc123" and connectrobot = "robot2"

    • PROBLEM: lecture_states["abc123"] = {} completely overwrites the existing data
    • Client A's data is lost! All sessions, state machines, etc. are gone
    • Sets global contents and time_list again, potentially with different content

Scenario 2: Concurrent Client Starts

If Client A and Client B start the same lecture simultaneously, they'll:

  • Race to access the same lecture_states dictionary
  • Overwrite each other's lecture state data
  • Override global content and timing data
  • Potentially corrupt each other's sessions

Code Analysis

Current Problematic Implementation

@router.post("/startLecture/{lecture_id}/{topic_id}")
async def start_lecture(lecture_id: str, topic_id: str, connectrobot: str = Form(...), db=Depends(get_db)):
    global topics, contents, time_list  # ❌ Global variables cause collisions
    
    # ... populate contents and time_list ...
    
    if lecture_id not in lecture_states:
        lecture_states[lecture_id] = {}  # ❌ Overwrites existing data
    
    if connectrobot not in lecture_states[lecture_id]:
        lecture_states[lecture_id][connectrobot] = {
            "state_machine": LectureStateMachine(lecture_id),
            "sessions": {}
        }
    
    # ... more code ...
    
    set_contents(contents)      # ❌ Global override!
    set_time_list(time_list)    # ❌ Global override!

The Shared Data Structure Issue

The lecture_states structure is:

lecture_states: Dict[str, Dict[str, Dict[str, Dict]]]
#           lecture_id → robot_id → session_id → session_data

But the current code does this:

if lecture_id not in lecture_states:
    lecture_states[lecture_id] = {}  # This is correct

if connectrobot not in lecture_states[lecture_id]:
    lecture_states[lecture_id][connectrobot] = {  # This is correct
        "state_machine": LectureStateMachine(lecture_id),
        "sessions": {}
    }

Impact Assessment

Data Corruption Risks

  • Session Loss: Active sessions from other clients are completely wiped out
  • State Machine Corruption: Lecture state machines are recreated, losing progress
  • Content Override: Global content and timing data gets overwritten
  • Race Conditions: Concurrent access leads to unpredictable behavior

User Experience Impact

  • Lecture Interruption: Active lectures may suddenly stop or restart
  • Data Loss: Student progress and Q&A sessions are lost
  • System Instability: Unpredictable behavior when multiple clients use the same lecture
  • Support Issues: Difficult to debug and reproduce problems

Recommended Solutions

1. Fix the Data Structure Overwrite

# CORRECT approach:
if lecture_id not in lecture_states:
    lecture_states[lecture_id] = {}

if connectrobot not in lecture_states[lecture_id]:
    lecture_states[lecture_id][connectrobot] = {
        "state_machine": LectureStateMachine(lecture_id),
        "sessions": {}
    }

2. Remove Global Variable Usage

# ❌ DON'T DO THIS:
global topics, contents, time_list
set_contents(contents)
set_time_list(time_list)

# ✅ DO THIS INSTEAD:
# Store content within each session context
lecture_states[lecture_id][connectrobot]["sessions"][session_id].update({
    "contents": contents,
    "time_list": time_list,
    "topics": topics
})

3. Implement Proper Session Isolation

# Each session should have its own isolated data
session_data = {
    "is_active": True,
    "selectedLanguageName": selected_language,
    "contents": contents,           # Session-specific content
    "time_list": time_list,        # Session-specific timing
    "connectrobot": connectrobot,
    "topics": topics,              # Session-specific topics
    "session_id": session_id
}

lecture_states[lecture_id][connectrobot]["sessions"][session_id] = session_data

4. Add Request Locking (Optional)

import asyncio

# Use asyncio.Lock to prevent concurrent modifications
lecture_locks = {}

async def start_lecture(lecture_id: str, topic_id: str, connectrobot: str = Form(...), db=Depends(get_db)):
    if lecture_id not in lecture_locks:
        lecture_locks[lecture_id] = asyncio.Lock()
    
    async with lecture_locks[lecture_id]:
        # ... lecture start logic ...
        pass

Implementation Priority

High Priority (Fix Immediately)

  1. Remove the lecture_states[lecture_id] = {} overwrite
  2. Eliminate global variable usage for lecture content
  3. Store all lecture data within session context

Medium Priority (Next Sprint)

  1. Implement proper error handling for concurrent access
  2. Add logging for lecture state changes
  3. Create unit tests for concurrent lecture starts

Low Priority (Future Enhancement)

  1. Implement distributed locking for multi-instance deployments
  2. Add monitoring and alerting for lecture state conflicts
  3. Create admin tools for managing active lecture sessions

Testing Recommendations

Test Scenarios

  1. Sequential Start: Start lecture with Client A, then start same lecture with Client B
  2. Concurrent Start: Start same lecture simultaneously with multiple clients
  3. Mixed Operations: Start different lectures with different clients, then start overlapping lectures
  4. Session Persistence: Verify that existing sessions remain intact when new sessions are created

Expected Behavior After Fix

  • Multiple clients can start the same lecture without conflicts
  • Each client maintains independent lecture state and content
  • No data corruption or session loss
  • Predictable and stable system behavior

Conclusion

The current lecture system implementation has a critical architectural flaw that will cause data corruption and session conflicts when multiple clients attempt to start the same lecture. This issue must be addressed immediately to ensure system stability and data integrity.

The fix involves:

  1. Proper data structure management (no overwriting existing data)
  2. Elimination of global variables (session-specific data storage)
  3. Implementation of proper isolation between different client sessions

Failure to address this issue will result in:

  • Unreliable system behavior
  • Data loss and corruption
  • Poor user experience
  • Increased support burden

Immediate action is required to prevent these issues from affecting production users.