Error handling is not thread safe and causing memory corruption and sporadic crashes

The problem is that not all compilers supported by OCCT do support thread_local keyword. Its use requires VC++ >= 14 of GCC >= 4.8... For CLang, the situation is unclear -- formally it supports this keyword since version 3.3, but Apple variant of the compiler used in XCode is said to not support it (at least till the very recent versions, perhaps 8.0).

Istvan, can you indicate what is your compiler, and confirm that it really does support thread_local?

We are using Xcode 8.2, and using the thread_local keyword works, and resolved the issues that were caused by this problem.

Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: abv
Date: Sun Dec 25 21:35:02 2016 +0300

    0028217: Error handling is not thread safe and causing memory corruption and sporadic crashes
    
    Static variable holding handle to the last raised exception is made thread-local on compilers that support C++11 keyword thread_local (MCVC++ 14+, GCC 4.8+, ICC 14+, CLang).

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Fix (working only for VC++ 14 and above, and GCC 4.8+) is pushed to CR28217; please review.

+// Define Standard_THREADLOCAL modifier as C++11 thread_local keyword
+// where it is available.
+#if (defined(__INTEL_COMPILER) && __INTEL_COMPILER > 1400) || \
+    (defined(__clang__)) /* assume standard CLang > 3.3 or XCode >= 8 */ || \
+    (defined(_MSC_VER) && _MSC_VER >= 1800) /* MSVC++ >= 14 */ || \
+    (defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8))) /* GCC >= 4.8 */
+  #define Standard_THREADLOCAL thread_local
+#else
+  #define Standard_THREADLOCAL
+#endif

Why this macros has been buried inside Standard_Failure.cxx, while it might be potentially useful in other places, and we place such attributes in Standard_Macro.hxx?

This macro has quite limited application: only the most modern compilers support thread_local variable. I see no other places where this macro could have been useful, hence kept it defined locally. The fix is not really universal but a kind of work-around to improve behavior for modern compilers only; the right fix will be to get rid of use of this static variable.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: apv
Date: Wed Dec 28 10:45:35 2016 +0300

    Test case tuning bugs/fclasses/bug28217

Dear Commenter 2,

Branch CR28217 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 63
   Windows: 0
   MacOS: 1127

Regressions/Differences:
Not detected

Testing cases:
bugs fclasses bug28217 - BAD (known problem)
http://occt-tests/CR28217-master-OCCT/Debian70-64/bugs/fclasses/bug28217.html
http://occt-tests/CR28217-master-OCCT/Windows-64-VC10/bugs/fclasses/bug28217.html

Testing on Linux:
Total MEMORY difference: 93289164 / 93045435 [+0.26%]
Total CPU difference: 21530.310000000223 / 21520.74000000037 [+0.04%]

Testing on Windows:
Total MEMORY difference: 58627848 / 58650770 [-0.04%]
Total CPU difference: 19406.0875971985 / 19731.318481998722 [-1.65%]

Branch [archived branch] has been deleted by Commenter 4.

[revision removed]