Bug#930570: garbled output when using ResourcePool from multiple threads

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Bug#930570: garbled output when using ResourcePool from multiple threads

Steinar H. Gunderson-2
Package: libmovit8
Version: 1.6.2-1
Severity: important
Tags: upstream

Hi,

I've noticed that occasionally in multithreaded contexts (which affects
at least Nageru on certain themes, but probably also Kdenlive, given its
heavy dependency on multiple threads), Movit will show corrupted imagery,
where textures from other threads show up at random.

I've traced this down to a bug in the multithread handling. From the upstream
commit message I wrote:

> When an EffectChain is done rendering (ie., has submitted all of the GL
> rendering commands that it needs to), it releases all of the temporary
> textures it's used back to a common freelist.
>
> However, if another thread needs a texture of the same size and format,
> it could be picking it off of the freelist before the GPU has actually
> completed rendering the first thread's command list, and start uploading
> into it. This is undefined behavior in OpenGL, and can create garbled
> output depending on timing and driver. (I've seen this on at least the
> classic Intel Mesa driver.)
>
> Fix by setting fences, so that getting a texture from the freelist
> will have an explicit ordering versus the previous work. This increases the
> size of ResourcePool::TextureFormat, but it is only ever used in a private
> std::map. std::map is node-based (it has to, since the C++ standard requires
> iterators to it to be stable), and thus, sizeof(TextureFormat) does not factor
> into sizeof(ResourcePool), and thus, there is no ABI break. Verified by
> checking on libstdc++.

The patch is small, doesn't break the ABI, and backports trivially from
upstream git. I'll be uploading through unstable, but it will probably be
stuck behind gcc-8 like Nageru was, so I assume eventually, a testing upload
will be required if the RMs accept it.

-- System Information:
Debian Release: 10.0
  APT prefers testing-proposed-updates
  APT policy: (500, 'testing-proposed-updates'), (500, 'testing-debug'), (500, 'testing'), (500, 'stable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.1.2 (SMP w/40 CPU cores)
Locale: LANG=en_DK.UTF-8, LC_CTYPE=en_DK.UTF-8 (charmap=UTF-8), LANGUAGE=en_NO:en_US:en_GB:en (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages libmovit8 depends on:
ii  libc6             2.28-10
ii  libepoxy0         1.5.3-0.1
ii  libfftw3-double3  3.3.8-2
ii  libgcc1           1:9.1.0-2
ii  libstdc++6        9.1.0-2

libmovit8 recommends no packages.

libmovit8 suggests no packages.

-- debconf-show failed