-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
In re-working the multi-threading for Windows, I believe I've come across a bounds issue. I discovered this when debugging my threading changes - a global static array was being overwritten. Debugging this showed the following:
In goto_set_num_threads() starting at line 571, there is the following code:
for(i = blas_num_threads - 1; i < num_threads - 1; i++) {
blas_threads[i] = CreateThread(NULL, 0, blas_thread_server, (void*)i, 0 &blas_threads_id[i]);
}The problem being that when goto_set_num_threads() is called at the start of the program, as the documentation recommends, blas_num_threads is 0. So the first thread ID is written to blas_threads[-1], causing that thread id to be lost and overwriting static blas_pool_t pool.
The issue doesn't cause an issue normally because the pool variable normally is not yet initialized when this happens. However, if you called goto_set_num_threads() or openblas_set_num_threads() after the server was initialized you would overwrite the pool.
I discovered this because the code I wrote for threading improvements added a free-list array of cached Event objects just before blas_threads to avoid excessive creation and destruction of Event objects. The end of this array was overwritten causing a race condition as threads were created, which usually resulted in a lost Event object.