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

Janus 0.7.2 crash when leaving videoroom #1685

Closed
pgj007 opened this issue Jul 5, 2019 · 16 comments
Closed

Janus 0.7.2 crash when leaving videoroom #1685

pgj007 opened this issue Jul 5, 2019 · 16 comments

Comments

@pgj007
Copy link

pgj007 commented Jul 5, 2019

(gdb) bt
#0  0x00007f3c6f5b9a1f in janus_videoroom_leave_or_unpublish () at /opt/janus/lib/janus/plugins/libjanus_videoroom.so
#1  0x00007f3c6f5dfc54 in janus_videoroom_handler () at /opt/janus/lib/janus/plugins/libjanus_videoroom.so
#2  0x00007f3c9523d4f0 in g_thread_proxy (data=0x96a8a0) at gthread.c:784
#3  0x00007f3c93d50dd5 in start_thread (arg=0x7f3c6b6dd700) at pthread_create.c:307
#4  0x00007f3c93a79ead in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
asm:
cmp    DWORD PTR [rbp-0x12c],0x0                                                                                                                                        
je     0x7f3c6f5b9a5d <janus_videoroom_leave_or_unpublish+1329>                                                                                                         
mov    rax,QWORD PTR [rbp-0x128]                                                                                                                                        
lea    rdx,[rax+0x18]                                                                                                                                                   
mov    rax,QWORD PTR [rbp-0x128]                                                                                                                                        
mov    rax,QWORD PTR [rax+0x8]                                                                                                                                          
mov    rax,QWORD PTR [rax+0xa8] //////////////////////////////////////////rax=0, so crash                                                                                                                                   
mov    rsi,rdx                                                                                                                                                          
mov    rdi,rax                                                                                                                                                          
call   0x7f3c6f5b25a0 <g_hash_table_remove@plt>                                                                                                                        
mov    rax,QWORD PTR [rbp-0x128]                                                                                                                                        
mov    eax,DWORD PTR [rax+0x20]                                                                                                                                         
mov    eax,eax                                                                                                                                                          
mov    rdx,rax                                                                                                                                                          
mov    rax,QWORD PTR [rbp-0x128]                                                                                                                                        
mov    rax,QWORD PTR [rax+0x8]                                                                                                                                          
mov    rax,QWORD PTR [rax+0xb0]                                                                                                                                         
mov    rsi,rdx                                                                                                                                                          
mov    rdi,rax                                                                                                                                                          
call   0x7f3c6f5b25a0 <g_hash_table_remove@plt>                                                                                                                         
mov    rax,QWORD PTR [rip+0x24251c]        # 0x7f3c6f7fbf80                                                                                                             
mov    eax,DWORD PTR [rax]                                                                                                                                              
test   eax,eax 
c:
g_hash_table_remove(participant->room->participants, &participant->user_id);
participant->room is null
@atoppi
Copy link
Member

atoppi commented Jul 8, 2019

Hi @pgj007,
are you sure participant->room is null? Sorry but I'm not so familiar with that asm code :-)

@pgj007
Copy link
Author

pgj007 commented Jul 9, 2019

@atoppi I confirm a crash at line 2375, room is the second element of janus_videoroom_publisher (mov rax,QWORD PTR [rax+0x8], now rax is room), and I suspect something is wrong with the lock.

@atoppi
Copy link
Member

atoppi commented Jul 11, 2019

Hi @pgj007,
the crash has been caused by this pointer being cleared when destroying the videoroom session.
In the same time, another thread (namely janus_videoroom_handler) is handling a leave request from the client.
What is the scenario that led to the crash? Is your client detaching and leaving concurrently? As a temporary workaround you may want to just detach the handle when leaving the videoroom.

@pgj007
Copy link
Author

pgj007 commented Jul 14, 2019

Hi @atoppi
I am exactly the same as your analysis, and my specific usage scenario is: the client has the function of reconnecting with broken wires, and when reconnecting, I will send "leave" first, which may be just when the session expires.

Your temporary solution is to send "detach" before "leave", am I correct?

As I am not familiar with janus, I need to look at it again. Thank you!

@atoppi
Copy link
Member

atoppi commented Jul 15, 2019

If you intend to leave a room and do not reuse the existing Janus handle, just send a detach message and Janus will take care of all of the logics related to leaving the room.

@pgj007
Copy link
Author

pgj007 commented Jul 16, 2019

@atoppi Yes, you're right. There's no need to leave. This problem you can write down first, can close first. Thank you for your answer.

@lminiero
Copy link
Member

Is it still an issue with master, or can we close this?

@akbarattar3
Copy link

this is still an issue

@atoppi
Copy link
Member

atoppi commented Sep 30, 2019

@pgj007 @akbarattar3 could you please test the PR #1795 ?

@lminiero
Copy link
Member

lminiero commented Oct 8, 2019

@pgj007 @akbarattar3 any update on what @atoppi asked? The patch has been around for more than a week and we haven't received any feedback. If you don't come back to us soon, we'll just assume it's fixed and merge the PR.

@akbarattar3
Copy link

private void closeInternal() {
Log.d(TAG, "Closing peer connection.");
statsTimer.cancel();

  1. /* if (peerConnectionMap != null) {
  2.         for (Map.Entry<BigInteger, JanusConnection> entry : peerConnectionMap.entrySet()) {
    
  3.             if (entry.getValue().peerConnection != null) {
    
  4.                 entry.getValue().peerConnection.dispose();
    
  5.             }
    
  6.         }
    
  7.         peerConnectionMap = null;
    
  8.     }*/
    
     Log.d(TAG, "Closing audio source.");
     if (audioSource != null) {
         audioSource.dispose();
         audioSource = null;
     }
     Log.d(TAG, "Stopping capture.");
     if (videoCapturer != null) {
         try {
             videoCapturer.stopCapture();
         } catch (InterruptedException e) {
             throw new RuntimeException(e);
         }
         videoCapturerStopped = true;
         videoCapturer.dispose();
         videoCapturer = null;
     }
     Log.d(TAG, "Closing video source.");
     if (videoSource != null) {
         videoSource.dispose();
         videoSource = null;
     }
     Log.d(TAG, "Closing peer connection factory.");
     if (factory != null) {
         //factory.dispose();
         factory = null;
     }
     options = null;
     Log.d(TAG, "Closing peer connection done.");
     events.onPeerConnectionClosed();
     PeerConnectionFactory.stopInternalTracingCapture();
     PeerConnectionFactory.shutdownInternalTracer();
    
    }
    I need to comment the 8 lines in close method then it stopped crashing .

@lminiero
Copy link
Member

lminiero commented Oct 9, 2019

Sorry, but I have no idea what you're talking about. That is not Janus code. Please only provide feedback on a clean Janus installation, as crashes happening on modified forks are out of scope.

@lminiero
Copy link
Member

lminiero commented Oct 9, 2019

Unless you mean that's your Android client? Is that crashing, or Janus?

@akbarattar3
Copy link

yeah Android client is crashing

@lminiero
Copy link
Member

lminiero commented Oct 9, 2019

Then it has nothing to do with this issue. We're only interested in figuring out if our patch fixes a crash in the videoroom.

@lminiero
Copy link
Member

lminiero commented Oct 9, 2019

Merged the fix in #1795, so I'll close this.

@lminiero lminiero closed this as completed Oct 9, 2019
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

No branches or pull requests

4 participants