blas_thread_init is not called for OS_WINNT

[cg31]

When using VS2019 to build OpenBlas, blas_thread_init needs to be called to initialize blas server. But current code only calls it for OS_CYGWIN_NT, this patch will fix it:

--- a/driver/others/blas_server_win32.c
+++ b/driver/others/blas_server_win32.c
@@ -348,9 +348,9 @@ int blas_thread_init(void){

 int exec_blas_async(BLASLONG pos, blas_queue_t *queue){

-#if defined(SMP_SERVER) && defined(OS_CYGWIN_NT)
+#if defined(SMP_SERVER) && (defined(OS_CYGWIN_NT) || defined(OS_WINNT))
   // Handle lazy re-init of the thread-pool after a POSIX fork
-  if (unlikely(blas_server_avail == 0)) blas_thread_init();
+  if (blas_server_avail == 0) blas_thread_init();
 #endif

[brada4]

Windows does not implement fork() like cygwin does, so that extra code path is absolutely impossible.
blas_thread_init is called at dllinit stage (you can drop to debugger at start of main() and see threads already around)

[brada4]

REF: https://docs.microsoft.com/en-us/cpp/porting/porting-from-unix-to-win32?view=vs-2019

[cg31]

I was talking about code in the master branch: https://github.com/xianyi/OpenBLAS/blob/develop/driver/others/blas_server_win32.c#L306

There is no fork() in it, it needs being called to initialize the threads, events and lock.

[brada4]

That code handles case possible case of fork() emulation in cygwin. The real thing happens in memory.c for "normal" windows case.

[martin-frbg]

@cg31 can you please describe the error you are getting without your patch ? It could be that the problem lies somewhere else and is only masked by this change.

[cg31]

@martin-frbg, without the patch, when exec_blas_async() is called, current->lock will be used without initialized.

[martin-frbg]

I believe the question is if this situation can occur in practice - normally the blas_thread_init() (with initialization of pool->lock) is expected to be called once (and early enough) from the gotoblas_init code in memory.c, and the additional safeguard in exec_blas_async would be only necessary on unix-like systems to initialize a new environment after a POSIX fork(). Do you see an actual crash without your
change, or is it just that VS2019 generates a warning that pool->lock "may be used before being defined" as the initialization happens in a different source file and code path ?)
( I assume you meant pool->lock here, as the InitializeCriticalSection() for current->lock happens in exec_blas_async itself ?)

[cg31]

Okay, it looks clearer now.

Firats of all, gotoblas_init is called in DllMain.

But when I used cmake to build OpenBLAS, I used -DMSVC_STATIC_CRT=ON to build it as a static library. DllMain will never be called, in turn, gotoblas_init will never be called.

In conclusion, this issue only happens when OpenBLAS is compiled and linked as static library. The better solution is to call gotoblas_init explicitly.

[martin-frbg]

Possibly related to #1652 ?

[cg31]

@martin-frbg , yes, it is related.

[embray]

Indeed, gotoblas_init is called (by way of DllMain), or should be, even openblas is compiled in statically via the (admittedly obscure) methods implemented here: https://github.com/xianyi/OpenBLAS/blob/2a43062de7d72f205d85cbc7f897ae35c20fada4/driver/others/memory.c#L1574

This strange, little-known incantation is explained well in this blog post: http://www.nynaeve.net/?p=183

[martin-frbg]

Thanks - unfortunately it is not quite clear to me what if anything a user needs to call in his program to
initialize a static OpenBLAS (something like _declspec(allocate "CRT$XLB") apparently ?), or what can be changed/fixed to make this unnecessary ? Perhaps I am just confused by the fact that the blog post appears to be mainly about TLS code, which is not the default in OpenBLAS' memory.c

[embray]

It just happens that doing things like initializing/freeing TLS slots is one of the main, if not the main use case for being able to run a callback when a thread starts or exits. So any mechanism for doing that tends to be intimately tied to TLS (similarly to how pthread_atfork is the interface to provide pre/post-fork callbacks even if you're not doing anything explicitly having to do with threads). Of course this mechanism can be exploited to run any arbitrary code during process/thread initialization.

I don't think the user shouldn't have to do anything in this case. If they statically link openblas into their binary these special sections should be linked into their binary as well (this would be worth checking though with objdump or dumpbin; perhaps there is some subtlety I don't know of to building this with VC 2019 such that it doesn't work as expected).

[embray]

I wonder, maybe, if the compiler doesn't arrange to actually add in the bits that the loader looks for to call these initialization callbacks if there isn't at least one variable explicitly declared with __declspec(thread). Will have to experiment with that (I've never actually tried this explicitly).

[brada4]

There is no loader for static binary.