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 --git a/resource_pool.cpp b/resource_pool.cpp
index af8380f47a884201176622c535e84bd8bdb87b77..7739d32790198349fba99bac3dd00910cbaf8788 100644 (file)
--- a/resource_pool.cpp
+++ b/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(GLint internal_format, GLsizei width, GLs
                    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(GLuint texture_num)
        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();

diff --git a/resource_pool.h b/resource_pool.h
index 85a5e52cd0de92a040bbd4f79f8e5e305aef5bf0..1b9515c09f4cfe21dc2db94f1f4c818ee84b4c4d 100644 (file)
--- a/resource_pool.h
+++ b/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