MantisBT - Community
View Issue Details
0028217Community[OCCT] OCCT:Foundation Classespublic2016-12-13 19:352017-10-19 14:04
Istvan Csanady 
apn 
normalcrash 
closedfixed 
 
[OCCT] 7.2.0[OCCT] 7.2.0 
bugs fclasses bug28217
0028217: Error handling is not thread safe and causing memory corruption and sporadic crashes
Standard_Failure.cxx:

static Handle(Standard_Failure) RaisedError = NULL;

Should be

thread_local Handle(Standard_Failure) RaisedError = NULL;

Currently this global variable could be set from different threads, causing crashes and possible memory corruption.

test bugs fclasses bug28217
No tags attached.
related to 0026937closed apn Community Eliminate NO_CXX_EXCEPTION macro support 
parent of 0028439closed apn Open CASCADE Configuration - compilation error when using thread_local within XCode 7 or earlier 
parent of 0029249closed bugmaster Open CASCADE Configuration - Standard_Failure compilation fails on VS2013 + Intel Compiler due to unavailability of thread_local 
Issue History
2016-12-13 19:35Istvan CsanadyNew Issue
2016-12-13 19:35Istvan CsanadyAssigned To => abv
2016-12-25 01:31abvRelationship addedrelated to 0026937
2016-12-25 19:06abvNote Added: 0062091
2016-12-25 19:10Istvan CsanadyNote Added: 0062092
2016-12-25 21:35gitNote Added: 0062095
2016-12-26 09:29gitNote Added: 0062099
2016-12-26 09:31abvNote Added: 0062100
2016-12-26 09:31abvStatusnew => resolved
2016-12-26 09:31abvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=15681#r15681
2016-12-26 09:31abvAssigned Toabv => kgv
2016-12-26 11:08kgvNote Added: 0062109
2016-12-26 19:03kgvAssigned Tokgv => abv
2016-12-26 19:03kgvStatusresolved => assigned
2016-12-27 08:58abvNote Added: 0062151
2016-12-27 08:59abvStatusassigned => resolved
2016-12-27 08:59abvAssigned Toabv => kgv
2016-12-27 09:13kgvAssigned Tokgv => bugmaster
2016-12-27 09:13kgvStatusresolved => reviewed
2016-12-27 14:18apvAssigned Tobugmaster => apv
2016-12-27 14:18apvTest case number => bugs fclasses bug28217
2016-12-28 10:46gitNote Added: 0062217
2016-12-28 11:23apvNote Added: 0062220
2016-12-28 11:23apvAssigned Toapv => bugmaster
2016-12-28 11:23apvStatusreviewed => tested
2016-12-30 18:08apnChangeset attached => occt master b3d20c7f
2016-12-30 18:08apnAssigned Tobugmaster => apn
2016-12-30 18:08apnStatustested => verified
2016-12-30 18:08apnResolutionopen => fixed
2017-02-03 19:59kgvRelationship addedparent of 0028439
2017-02-08 11:32gitNote Added: 0063561
2017-09-29 16:21aivFixed in Version => 7.2.0
2017-09-29 16:31aivStatusverified => closed
2017-10-19 14:00kgvRelationship addedrelated to 0029198
2017-10-19 14:04kgvRelationship addedparent of 0029249

Notes
(0062091)
abv   
2016-12-25 19:06   
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?
(0062092)
Istvan Csanady   
2016-12-25 19:10   
We are using Xcode 8.2, and using the thread_local keyword works, and resolved the issues that were caused by this problem.
(0062095)
git   
2016-12-25 21:35   
Branch CR28217 has been created by abv.

SHA-1: 493caea78ed98f69b7c1e1784ef2dc0f49f1466a


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).
(0062099)
git   
2016-12-26 09:29   
Branch CR28217 has been updated forcibly by abv.

SHA-1: 1633e77ece69fcf1d07c8314e2efe1158b8c6f2a
(0062100)
abv   
2016-12-26 09:31   
Fix (working only for VC++ 14 and above, and GCC 4.8+) is pushed to CR28217; please review.
(0062109)
kgv   
2016-12-26 11:08   
+// 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?
(0062151)
abv   
2016-12-27 08:58   
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.
(0062217)
git   
2016-12-28 10:46   
Branch CR28217 has been updated by apv.

SHA-1: 83a7db8847fc584049e03868c1cb25361fa55415


Detailed log of new commits:

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

    Test case tuning bugs/fclasses/bug28217

(0062220)
apv   
2016-12-28 11:23   
Dear BugMaster,

Branch CR28217 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 1633e77ece69fcf1d07c8314e2efe1158b8c6f2a

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%]
(0063561)
git   
2017-02-08 11:32   
Branch CR28217 has been deleted by kgv.

SHA-1: 83a7db8847fc584049e03868c1cb25361fa55415