Texture object locking

[Neil Roberts]

Jason wrote:

> Kristian and I were looking at this today, and there seems to be a
> substantial race in the way that we are doing texture locking.

Yes, it does look like this is a problem. I also noticed another
dodgy-looking pattern when implementing the GL_ARB_clear_texture
extension. Whenever it enters a meta function that operates on a
texture image it unlocks the texture object and then does stuff
directly using the texture image pointer. Presumably that pointer
could also be freed at any point. Take a look at
copytexsubimage_using_blit_framebuffer in meta.c for an example.

I suppose in practice though it's probably not all that likely that an
application will go and delete textures in another thread without the
other threads expecting it so I don't know whether it's a major
concern.

I knocked up the following patch to Weston's simple-egl example to try
and get it to crash when deleting textures from multiple threads and
sure enough it does segfault.

Regards,
- Neil

------- >8 --------------- (use git am --scissors to automatically chop here)
Subject: Hack to test multi-threaded deleting textures

---
 clients/simple-egl.c | 125 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 112 insertions(+), 13 deletions(-)

diff --git a/clients/simple-egl.c b/clients/simple-egl.c
index 2097b4c..6e592b4 100644
--- a/clients/simple-egl.c
+++ b/clients/simple-egl.c
@@ -39,6 +39,7 @@
 #include <GLES2/gl2.h>
 #include <EGL/egl.h>
 #include <EGL/eglext.h>
+#include <pthread.h>
 
 #include "xdg-shell-client-protocol.h"
 
@@ -100,6 +101,13 @@ struct window {
 	int fullscreen, opaque, buffer_size, frame_sync;
 };
 
+struct thread_data {
+	EGLDisplay dpy;
+	EGLSurface surface;
+	EGLContext ctx;
+	int thread_num;
+};
+
 static const char *vert_shader_text =
 	"uniform mat4 rotation;\n"
 	"attribute vec4 pos;\n"
@@ -117,15 +125,25 @@ static const char *frag_shader_text =
 	"  gl_FragColor = v_color;\n"
 	"}\n";
 
+static const EGLint context_attribs[] = {
+	EGL_CONTEXT_CLIENT_VERSION, 2,
+	EGL_NONE
+};
+
 static int running = 1;
 
+#define N_THREADS 63
+#define N_TEXTURES 512
+#define TEX_SIZE 128
+
+static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+static uint64_t running_threads;
+static GLuint textures[N_TEXTURES];
+
 static void
 init_egl(struct display *display, struct window *window)
 {
-	static const EGLint context_attribs[] = {
-		EGL_CONTEXT_CLIENT_VERSION, 2,
-		EGL_NONE
-	};
 	const char *extensions;
 
 	EGLint config_attribs[] = {
@@ -748,13 +766,101 @@ usage(int error_code)
 	exit(error_code);
 }
 
+static GLuint
+create_texture(void)
+{
+	GLuint tex;
+	GLubyte data[TEX_SIZE * TEX_SIZE * 4];
+
+	glGenTextures(1, &tex);
+	glBindTexture(GL_TEXTURE_2D, tex);
+	glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, TEX_SIZE, TEX_SIZE, 0,
+		     GL_RGBA, GL_UNSIGNED_BYTE, data);
+	glBindTexture(GL_TEXTURE_2D, 0);
+
+	return tex;
+}
+
+static void *
+thread_func(void *data)
+{
+	struct thread_data *thread_data = data;
+	int i;
+
+	eglMakeCurrent(thread_data->dpy,
+		       thread_data->surface,
+		       thread_data->surface,
+		       thread_data->ctx);
+
+	while (true) {
+		/* Wait until this thread is told to run */
+		pthread_mutex_lock(&mutex);
+		while (!(running_threads & (1ULL << thread_data->thread_num)))
+			pthread_cond_wait(&cond, &mutex);
+		pthread_mutex_unlock(&mutex);
+
+		/* Delete all of the textures */
+		for (i = 0; i < N_TEXTURES; i++)
+			glDeleteTextures(1, textures + i);
+
+		/* Report that our thread is no longer running */
+		pthread_mutex_lock(&mutex);
+		running_threads &= ~(1ULL << thread_data->thread_num);
+		pthread_cond_broadcast(&cond);
+		pthread_mutex_unlock(&mutex);
+	}
+
+	return data;
+}
+
+static void
+delete_thread_test(struct display *display)
+{
+	pthread_t thread;
+	struct thread_data thread_data[N_THREADS];
+	int i;
+
+	for (i = 0; i < N_THREADS; i++) {
+		thread_data[i].dpy = display->egl.dpy;
+		thread_data[i].surface = display->window->egl_surface;
+		thread_data[i].ctx = eglCreateContext(display->egl.dpy,
+						      display->egl.conf,
+						      display->egl.ctx,
+						      context_attribs);
+		thread_data[i].thread_num = i;
+		pthread_create(&thread,
+			       NULL, /* attr */
+			       thread_func,
+			       thread_data + i);
+	}
+
+	pthread_mutex_lock(&mutex);
+
+	while (true) {
+		/* Wait until all of the threads have finished trying
+		 * to delete the textures */
+		while (running_threads)
+			pthread_cond_wait(&cond, &mutex);
+
+		/* Create some textures to delete */
+		for (i = 0; i < N_TEXTURES; i++)
+			textures[i] = create_texture();
+
+		/* Set all the threads running */
+		running_threads = (1ULL << N_THREADS) - 1;
+		pthread_cond_broadcast(&cond);
+	}
+
+	pthread_mutex_unlock(&mutex);
+}
+
 int
 main(int argc, char **argv)
 {
 	struct sigaction sigint;
 	struct display display = { 0 };
 	struct window  window  = { 0 };
-	int i, ret = 0;
+	int i;
 
 	window.display = &display;
 	display.window = &window;
@@ -800,14 +906,7 @@ main(int argc, char **argv)
 	sigint.sa_flags = SA_RESETHAND;
 	sigaction(SIGINT, &sigint, NULL);
 
-	/* The mainloop here is a little subtle.  Redrawing will cause
-	 * EGL to read events so we can just call
-	 * wl_display_dispatch_pending() to handle any events that got
-	 * queued up as a side effect. */
-	while (running && ret != -1) {
-		wl_display_dispatch_pending(display.display);
-		redraw(&window, NULL, 0);
-	}
+	delete_thread_test(&display);
 
 	fprintf(stderr, "simple-egl exiting\n");
 
-- 
1.9.3