(I was trying to be too clever with git-sendmail and I accidentally sent this to the wrong list… sorry!)
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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev