MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0028217Community[OCCT] OCCT:Foundation Classespublic2016-12-13 19:352017-10-19 14:04
ReporterIstvan Csanady 
Assigned Toapn 
PrioritynormalSeveritycrash 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.2.0Fixed in Version[OCCT] 7.2.0 
Summary0028217: Error handling is not thread safe and causing memory corruption and sporadic crashes
DescriptionStandard_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.

Steps To Reproducetest bugs fclasses bug28217
TagsNo tags attached.
Test case numberbugs fclasses bug28217
Attached Files

- Relationships
related to 0026937closedapn Community Eliminate NO_CXX_EXCEPTION macro support 
parent of 0028439closedapn Open CASCADE Configuration - compilation error when using thread_local within XCode 7 or earlier 
parent of 0029249closedbugmaster Open CASCADE Configuration - Standard_Failure compilation fails on VS2013 + Intel Compiler due to unavailability of thread_local 

-  Notes
(0062091)
abv (manager)
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 (developer)
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 (administrator)
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 (administrator)
2016-12-26 09:29

Branch CR28217 has been updated forcibly by abv.

SHA-1: 1633e77ece69fcf1d07c8314e2efe1158b8c6f2a
(0062100)
abv (manager)
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 (developer)
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 (manager)
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 (administrator)
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 (tester)
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 (administrator)
2017-02-08 11:32

Branch CR28217 has been deleted by kgv.

SHA-1: 83a7db8847fc584049e03868c1cb25361fa55415

- Related Changesets
occt: master b3d20c7f
Timestamp: 2016-12-29 11:37:42
Author: abv
Committer: apn
Details ] Diff ]
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).
Test bugs fclasses bug28217 is added (BAD on vc < 14 and gcc < 4.8).

Test case tuning bugs/fclasses/bug28217
mod - src/QABugs/QABugs_20.cxx Diff ] File ]
mod - src/Standard/Standard_Failure.cxx Diff ] File ]
add - tests/bugs/fclasses/bug28217 Diff ] File ]

- Issue History
Date Modified Username Field Change
2016-12-13 19:35 Istvan Csanady New Issue
2016-12-13 19:35 Istvan Csanady Assigned To => abv
2016-12-25 01:31 abv Relationship added related to 0026937
2016-12-25 19:06 abv Note Added: 0062091
2016-12-25 19:10 Istvan Csanady Note Added: 0062092
2016-12-25 21:35 git Note Added: 0062095
2016-12-26 09:29 git Note Added: 0062099
2016-12-26 09:31 abv Note Added: 0062100
2016-12-26 09:31 abv Status new => resolved
2016-12-26 09:31 abv Steps to Reproduce Updated View Revisions
2016-12-26 09:31 abv Assigned To abv => kgv
2016-12-26 11:08 kgv Note Added: 0062109
2016-12-26 19:03 kgv Assigned To kgv => abv
2016-12-26 19:03 kgv Status resolved => assigned
2016-12-27 08:58 abv Note Added: 0062151
2016-12-27 08:59 abv Status assigned => resolved
2016-12-27 08:59 abv Assigned To abv => kgv
2016-12-27 09:13 kgv Assigned To kgv => bugmaster
2016-12-27 09:13 kgv Status resolved => reviewed
2016-12-27 14:18 apv Assigned To bugmaster => apv
2016-12-27 14:18 apv Test case number => bugs fclasses bug28217
2016-12-28 10:46 git Note Added: 0062217
2016-12-28 11:23 apv Note Added: 0062220
2016-12-28 11:23 apv Assigned To apv => bugmaster
2016-12-28 11:23 apv Status reviewed => tested
2016-12-30 18:08 apn Changeset attached => occt master b3d20c7f
2016-12-30 18:08 apn Assigned To bugmaster => apn
2016-12-30 18:08 apn Status tested => verified
2016-12-30 18:08 apn Resolution open => fixed
2017-02-03 19:59 kgv Relationship added parent of 0028439
2017-02-08 11:32 git Note Added: 0063561
2017-09-29 16:21 user533 Fixed in Version => 7.2.0
2017-09-29 16:31 user533 Status verified => closed
2017-10-19 14:00 kgv Relationship added related to 0029198
2017-10-19 14:04 kgv Relationship added parent of 0029249


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker