Testing xbmc pvr; libGL threading issues...

[Juergen Lock]

On Fri, Oct 14, 2011 at 11:54:50PM +0200, Juergen Lock wrote:
> Hi!
> 
>  I had been playing with opdenkamp's xbmc pvr branch and pipelka's
> xvdr xbmc addon and vdr plugin for a little while now, and finally
> have something that more or less runs.  One issue that took me a
> while to figure out were crashes in libGL when switching live
> channels:
> 
> (gdb) bt
> #0  0x00000008041abca7 in glIsTexture () from /usr/local/lib/libGL.so.1
> #1  0x0000000000b2a995 in CLinuxRendererGL::DeleteYV12Texture (
>     this=0x8340b8800, index=0) at LinuxRendererGL.cpp:1690
> #2  0x0000000000b2eae2 in CLinuxRendererGL::UnInit (this=0x8340b8800)
>     at LinuxRendererGL.cpp:1059
> #3  0x0000000000b2ed9e in CLinuxRendererGL::PreInit (this=0x8340b8800)
>     at LinuxRendererGL.cpp:719
> #4  0x0000000000b29049 in CXBMCRenderManager::PreInit (this=0x1546920)
>     at RenderManager.cpp:324
> #5  0x0000000000b8bf4f in CDVDPlayerVideo::OpenStream (this=0x835400590, 
>     hint=@0x7ffffdfcee20) at DVDPlayerVideo.cpp:183
> #6  0x0000000000b6d88e in CDVDPlayer::OpenVideoStream (this=0x835400000, 
>     iStream=0, source=256) at DVDPlayer.cpp:2891
> #7  0x0000000000b79734 in CDVDPlayer::Process (this=0x835400000)
>     at DVDPlayer.cpp:1207
> #8  0x0000000000f4a74d in CThread::staticThread (data=0x835400010)
>     at Thread.cpp:177
> 
> turns out libGL keeps thread-local pointers and xbmc called into
> libGL (glIsTexture()) from (apparently) a new thread where then
> _x86_64_get_dispatch in:
> 
> 	work/Mesa-7.6.1/src/mesa/x86-64/glapi_x86-64.S
> 
> returned NULL and thus the glIsTexture wrapper in the same .S
> got a segfault referencing that NULL pointer here:
> 
> [...]
>         .p2align        4,,15
>         .globl  GL_PREFIX(IsTexture)
>         .type   GL_PREFIX(IsTexture), @function
> GL_PREFIX(IsTexture):
> #if defined(GLX_USE_TLS)
>         call    _x86_64_get_dispatch at PLT
>         movq    2640(%rax), %r11
>         jmp     *%r11
> #elif defined(PTHREADS)
>         pushq   %rdi
>         call    _x86_64_get_dispatch at PLT
>         popq    %rdi
>         movq    2640(%rax), %r11
> 	^^^^^^^^^^^^^^^^^^^^^^^^ segfault
>         jmp     *%r11
> #else
> [...]
> 
> _x86_64_get_dispatch uses pthread_getspecific() and looks like this:
> 
> [...]
> #elif defined(PTHREADS)
> 
>         .extern _glapi_Dispatch
>         .extern _gl_DispatchTSD
>         .extern pthread_getspecific
> 
>         .p2align        4,,15
> _x86_64_get_dispatch:
>         movq    _gl_DispatchTSD at GOTPCREL(%rip), %rax
>         movl    (%rax), %edi
>         jmp     pthread_getspecific at PLT
> 
> #elif defined(THREADS)
> [...]
> 
> and some documentation about these thread-local pointers is here:
> 
> 	work/Mesa-7.6.1/docs/dispatch.html
> 
>  After some googling I stumbled across this debian ticket that has a patch:
> 
> 	http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=536106
> 
> so I changed the libGL port's files/patch-src__mesa__x86-64__glapi_x86-64.S
> into this:
> 
> --------snip------
> --- ./src/mesa/x86-64/glapi_x86-64.S.orig	2009-03-13 04:28:49.000000000 +0100
> +++ ./src/mesa/x86-64/glapi_x86-64.S	2011-01-28 18:12:18.000000000 +0100
> @@ -73,7 +73,12 @@ _x86_64_get_dispatch:
>  
>  	.p2align	4,,15
>  _x86_64_get_dispatch:
> -	movq	_gl_DispatchTSD(%rip), %rdi
> +	movq	_glapi_Dispatch(%rip), %rax
> +	testq	%rax,%rax
> +	je	1f
> +	ret
> +1:	movq	_gl_DispatchTSD at GOTPCREL(%rip), %rax
> +	movl	(%rax), %edi
>  	jmp	pthread_getspecific at PLT
>  
>  #elif defined(THREADS)
> --------snip------
> 
>  That fixed these crashes but now I got other crashes that seem to be
> related to messages like:
> 
> 	Recursive call into r300FlushCmdBufLocked!
> 
> (this is using the xorg radeon drivers, the nvidia libGL blob on
> the other box crashed in glIsTexture() too but obviously I cannot
> easily debug that) - so I looked for another workaround.  What I
> finally ended up doing was this patch:
> (files/patch-xbmc-cores-VideoRenderers-LinuxRendererGL.cpp)
> 
> --- xbmc/cores/VideoRenderers/LinuxRendererGL.cpp.orig
> +++ xbmc/cores/VideoRenderers/LinuxRendererGL.cpp
> @@ -159,6 +159,10 @@ CLinuxRendererGL::CLinuxRendererGL()
>    m_rgbPbo = 0;
>  
>    m_dllSwScale = new DllSwScale;
> +
> +#ifdef __FreeBSD__
> +  m_tid = NULL;
> +#endif
>  }
>  
>  CLinuxRendererGL::~CLinuxRendererGL()
> @@ -247,6 +251,9 @@ bool CLinuxRendererGL::ValidateRenderTar
>        (this->*m_textureCreate)(i);
>  
>      m_bValidated = true;
> +#ifdef __FreeBSD__
> +    m_tid = pthread_self();
> +#endif
>      return true;
>    }
>    return false;
> @@ -716,6 +723,9 @@ unsigned int CLinuxRendererGL::PreInit()
>    CSingleLock lock(g_graphicsContext);
>    m_bConfigured = false;
>    m_bValidated = false;
> +#ifdef __FreeBSD__ // XXX Will this leak?  It's needed to avoid crashes... :(
> +  if (pthread_self() == m_tid)
> +#endif
>    UnInit();
>    m_resolution = g_guiSettings.m_LookAndFeelResolution;
>    if ( m_resolution == RES_WINDOW )
> 
>  i.e. I only called into CLinuxRendererGL::UnInit() (where the original
> crashes happened) if this was the same thread that created the textures
> that CLinuxRendererGL::UnInit() tried to delete.  I don't know if this
> will leak memory, textures, or something else so I'd prefer libGL
> to be fixed properly...  (And as you might have guessed, this code runs
> just fine unpatched on Linux...)

And now I tested this with libGL and friends from xorg-dev,

	http://wiki.freebsd.org/Xorg

and those no longer need this patch.  (I didn't update all of xorg,
just the stuff below graphics/ .)

 Just FYI, :)
	Juergen