Package: release.debian.org Severity: normal User: release.debian....@packages.debian.org Usertags: unblock
Please unblock package movit It contains a single, focused fix for a severity=important bug that I found, backported from upstream git. It's a bit unclear to me whether “corrupted display” would count as appropriate for buster unblocks, but I thought it might go under the “crashes etc.” heading (it's technically undefined behavior with a thread race, but I don't think there are any real GL drivers where this actually causes _crashes_ per se). If not, please let me know. From the upstream commit message; especially the part about ABI stability at the end should be relevant: Fix an issue where temporary textures could be reused too early by a different thread. 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++. diff -Nru movit-1.6.2/debian/changelog movit-1.6.2/debian/changelog --- movit-1.6.2/debian/changelog 2018-03-18 16:25:30.000000000 +0100 +++ movit-1.6.2/debian/changelog 2019-06-15 20:08:45.000000000 +0200 @@ -1,3 +1,11 @@ +movit (1.6.2-2) unstable; urgency=high + + * fix-temporary-texture-race-issue.diff: New patch backported from upstream + git, fixes a threading issue where a thread could start reusing a temporary + texture while it's still being used in rendering. (Closes: #930570) + + -- Steinar H. Gunderson <se...@debian.org> Sat, 15 Jun 2019 20:08:45 +0200 + movit (1.6.2-1) unstable; urgency=medium * New upstream release. diff -Nru movit-1.6.2/debian/patches/fix-temporary-texture-race-issue.diff movit-1.6.2/debian/patches/fix-temporary-texture-race-issue.diff --- movit-1.6.2/debian/patches/fix-temporary-texture-race-issue.diff 1970-01-01 01:00:00.000000000 +0100 +++ movit-1.6.2/debian/patches/fix-temporary-texture-race-issue.diff 2019-06-15 20:08:37.000000000 +0200 @@ -0,0 +1,50 @@ +Index: movit-1.6.2/resource_pool.cpp +=================================================================== +--- movit-1.6.2.orig/resource_pool.cpp ++++ movit-1.6.2/resource_pool.cpp +@@ -42,6 +42,7 @@ ResourcePool::~ResourcePool() + for (GLuint free_texture_num : texture_freelist) { + assert(texture_formats.count(free_texture_num) != 0); + texture_freelist_bytes -= estimate_texture_size(texture_formats[free_texture_num]); ++ glDeleteSync(texture_formats[free_texture_num].no_reuse_before); + texture_formats.erase(free_texture_num); + glDeleteTextures(1, &free_texture_num); + check_error(); +@@ -337,7 +338,10 @@ GLuint ResourcePool::create_2d_texture(G + format_it->second.height == height) { + texture_freelist_bytes -= estimate_texture_size(format_it->second); + texture_freelist.erase(freelist_it); ++ GLsync sync = format_it->second.no_reuse_before; + pthread_mutex_unlock(&lock); ++ glWaitSync(sync, 0, GL_TIMEOUT_IGNORED); ++ glDeleteSync(sync); + return texture_num; + } + } +@@ -449,12 +453,14 @@ void ResourcePool::release_2d_texture(GL + texture_freelist.push_front(texture_num); + assert(texture_formats.count(texture_num) != 0); + texture_freelist_bytes += estimate_texture_size(texture_formats[texture_num]); ++ texture_formats[texture_num].no_reuse_before = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); + + while (texture_freelist_bytes > texture_freelist_max_bytes) { + GLuint free_texture_num = texture_freelist.back(); + texture_freelist.pop_back(); + assert(texture_formats.count(free_texture_num) != 0); + texture_freelist_bytes -= estimate_texture_size(texture_formats[free_texture_num]); ++ glDeleteSync(texture_formats[free_texture_num].no_reuse_before); + texture_formats.erase(free_texture_num); + glDeleteTextures(1, &free_texture_num); + check_error(); +Index: movit-1.6.2/resource_pool.h +=================================================================== +--- movit-1.6.2.orig/resource_pool.h ++++ movit-1.6.2/resource_pool.h +@@ -211,6 +211,7 @@ private: + struct Texture2D { + GLint internal_format; + GLsizei width, height; ++ GLsync no_reuse_before = nullptr; + }; + + // A mapping from texture number to format details. This is filled if the diff -Nru movit-1.6.2/debian/patches/series movit-1.6.2/debian/patches/series --- movit-1.6.2/debian/patches/series 1970-01-01 01:00:00.000000000 +0100 +++ movit-1.6.2/debian/patches/series 2019-06-15 20:08:08.000000000 +0200 @@ -0,0 +1 @@ +fix-temporary-texture-race-issue.diff unblock movit/1.6.2-2 -- 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.11 (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)