Skip to content

Commit d209667

Browse files
committed
compat/w32pthreads: change pthread_t into pointer to malloced struct
pthread_t is currently defined as a struct, which gets placed into caller's memory and filled by pthread_create() (which accepts a pthread_t*). The problem with this approach is that pthread_join() accepts pthread_t itself rather than a pointer to it, so it gets a _copy_ of this structure. This causes non-deterministic failures of pthread_join() to produce the correct return value - depending on whether the thread already finished before pthread_join() is called (and thus the copy contains the correct value), or not (then it contains 0). Change the definition of pthread_t into a pointer to a struct, that gets malloced by pthread_create() and freed by pthread_join(). Fixes random failures of fate-ffmpeg-error-rate-fail on Windows after 433cf39. See also [1] for an alternative approach that does not require dynamic allocation, but relies on an assumption that the pthread_t value remains in a fixed memory location. [1] https://code.videolan.org/videolan/x264/-/commit/23829dd2b2c909855481f46cc884b3c25d92c2d7 Reviewed-By: Martin Storsjö <[email protected]>
1 parent 17e4746 commit d209667

File tree

1 file changed

+27
-12
lines changed

1 file changed

+27
-12
lines changed

compat/w32pthreads.h

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ typedef struct pthread_t {
5050
void *(*func)(void* arg);
5151
void *arg;
5252
void *ret;
53-
} pthread_t;
53+
} *pthread_t;
5454

5555
/* use light weight mutex/condition variable API for Windows Vista and later */
5656
typedef SRWLOCK pthread_mutex_t;
@@ -74,38 +74,53 @@ typedef CONDITION_VARIABLE pthread_cond_t;
7474
static av_unused THREADFUNC_RETTYPE
7575
__stdcall attribute_align_arg win32thread_worker(void *arg)
7676
{
77-
pthread_t *h = (pthread_t*)arg;
77+
pthread_t h = (pthread_t)arg;
7878
h->ret = h->func(h->arg);
7979
return 0;
8080
}
8181

8282
static av_unused int pthread_create(pthread_t *thread, const void *unused_attr,
8383
void *(*start_routine)(void*), void *arg)
8484
{
85-
thread->func = start_routine;
86-
thread->arg = arg;
85+
pthread_t ret;
86+
87+
ret = av_mallocz(sizeof(*ret));
88+
if (!ret)
89+
return EAGAIN;
90+
91+
ret->func = start_routine;
92+
ret->arg = arg;
8793
#if HAVE_WINRT
88-
thread->handle = (void*)CreateThread(NULL, 0, win32thread_worker, thread,
89-
0, NULL);
94+
ret->handle = (void*)CreateThread(NULL, 0, win32thread_worker, ret,
95+
0, NULL);
9096
#else
91-
thread->handle = (void*)_beginthreadex(NULL, 0, win32thread_worker, thread,
92-
0, NULL);
97+
ret->handle = (void*)_beginthreadex(NULL, 0, win32thread_worker, ret,
98+
0, NULL);
9399
#endif
94-
return !thread->handle;
100+
101+
if (!ret->handle) {
102+
av_free(ret);
103+
return EAGAIN;
104+
}
105+
106+
*thread = ret;
107+
108+
return 0;
95109
}
96110

97111
static av_unused int pthread_join(pthread_t thread, void **value_ptr)
98112
{
99-
DWORD ret = WaitForSingleObject(thread.handle, INFINITE);
113+
DWORD ret = WaitForSingleObject(thread->handle, INFINITE);
100114
if (ret != WAIT_OBJECT_0) {
101115
if (ret == WAIT_ABANDONED)
102116
return EINVAL;
103117
else
104118
return EDEADLK;
105119
}
106120
if (value_ptr)
107-
*value_ptr = thread.ret;
108-
CloseHandle(thread.handle);
121+
*value_ptr = thread->ret;
122+
CloseHandle(thread->handle);
123+
av_free(thread);
109124
return 0;
110125
}
111126

0 commit comments

Comments
 (0)