From bc5dd8dd0f10724184aa32cd5ee895251672ab6c Mon Sep 17 00:00:00 2001
From: Luke Wilde <lukew@serenityos.org>
Date: Wed, 1 Jun 2022 14:38:58 +0100
Subject: [PATCH] LibGL: Check that texture name is allocated before marking it
 as free

glDeleteTextures previously did not check that the texture name was
allocated by glGenTextures before adding it to the free texture name
list.

This means that if you delete a texture twice in a row, the name will
appear twice in the free texture list, making glGenTextures return the
same texture name twice in a row.
---
 Tests/LibGL/CMakeLists.txt            |  1 +
 Tests/LibGL/TestAPI.cpp               | 42 +++++++++++++++++++++++++++
 Userland/Libraries/LibGL/Textures.cpp |  5 ++--
 3 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 Tests/LibGL/TestAPI.cpp

diff --git a/Tests/LibGL/CMakeLists.txt b/Tests/LibGL/CMakeLists.txt
index d29a58ae71..02e5440530 100644
--- a/Tests/LibGL/CMakeLists.txt
+++ b/Tests/LibGL/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(TEST_SOURCES
+    TestAPI.cpp
     TestRender.cpp
 )
 
diff --git a/Tests/LibGL/TestAPI.cpp b/Tests/LibGL/TestAPI.cpp
new file mode 100644
index 0000000000..c33ecf108f
--- /dev/null
+++ b/Tests/LibGL/TestAPI.cpp
@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2022, Luke Wilde <lukew@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibGL/GL/gl.h>
+#include <LibGL/GLContext.h>
+#include <LibTest/TestCase.h>
+
+static NonnullOwnPtr<GL::GLContext> create_testing_context()
+{
+    auto bitmap = MUST(Gfx::Bitmap::try_create(Gfx::BitmapFormat::BGRx8888, { 1, 1 }));
+    auto context = GL::create_context(*bitmap);
+    GL::make_context_current(context);
+    return context;
+}
+
+TEST_CASE(0001_gl_gen_textures_does_not_return_the_same_texture_name_twice_unless_deleted)
+{
+    // https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/glGenTextures.xhtml
+    // "Texture names returned by a call to glGenTextures are not returned by subsequent calls, unless they are first deleted with glDeleteTextures."
+    auto context = create_testing_context();
+
+    GLuint texture1 = 0;
+    GLuint texture2 = 0;
+
+    glGenTextures(1, &texture1);
+
+    // glDeleteTextures previously did not check that the texture name was allocated by glGenTextures before adding it to the free texture name list.
+    // This means that if you delete a texture twice in a row, the name will appear twice in the free texture list, making glGenTextures return the
+    // same texture name twice in a row.
+    glDeleteTextures(1, &texture1);
+    glDeleteTextures(1, &texture1);
+
+    texture1 = 0;
+
+    glGenTextures(1, &texture1);
+    glGenTextures(1, &texture2);
+
+    EXPECT_NE(texture1, texture2);
+}
diff --git a/Userland/Libraries/LibGL/Textures.cpp b/Userland/Libraries/LibGL/Textures.cpp
index dab82c0804..019ba87d35 100644
--- a/Userland/Libraries/LibGL/Textures.cpp
+++ b/Userland/Libraries/LibGL/Textures.cpp
@@ -107,11 +107,12 @@ void GLContext::gl_delete_textures(GLsizei n, GLuint const* textures)
         if (name == 0)
             continue;
 
-        m_name_allocator.free(name);
-
         auto texture_object = m_allocated_textures.find(name);
         if (texture_object == m_allocated_textures.end() || texture_object->value.is_null())
             continue;
+
+        m_name_allocator.free(name);
+
         auto texture = texture_object->value;
 
         // Check all texture units