MantisBT - Open CASCADE
View Issue Details
0027620Open CASCADE[OCCT] OCCT:DRAWpublic2016-06-20 11:132018-11-26 10:36
abv 
apn 
normalmajor 
verifiedfixed 
 
[OCCT] 7.4.0* 
perf bop boxholes
0027620: Test perf bop boxholes crashes DRAW
When test bop boxholes is run twice, DRAW exits without any message (current master built with VS 2015, 64-bit, Release mode, without TBB).

On another machine, it crashes immediately (VS 2010, 64-bit).

OCCT 7.0 installed from official distributive (VS 2010 version) crashes immediately.
Problem 1:
> test perf bop boxholes

Problem 2:
> for {set i 0} {$i < 1000000} {incr i} { dlog on; box b 1 1 1; dlog off; puts $i }
No tags attached.
related to 0027043verified bugmaster Open CASCADE Draw is sometimes crashed unexpectedly during running of test command 
related to 0024012closed ifv Community problem with BRepAlgo_NormalProjection 
Issue History
2016-06-20 11:13abvNew Issue
2016-06-20 11:13abvAssigned To => msv
2016-06-20 15:27msvRelationship addedrelated to 0027043
2016-06-20 15:35abvAssigned Tomsv => abv
2016-10-28 16:28msvTarget Version7.1.0 => 7.2.0
2017-07-24 09:32msvTarget Version7.2.0 => 7.3.0
2017-08-16 19:13apvTest case number => perf bop boxholes
2017-08-16 19:14apvNote Added: 0069490
2017-12-05 16:59msvTarget Version7.3.0 => 7.4.0*
2018-11-10 08:25abvNote Added: 0081018
2018-11-11 21:12abvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=20365#r20365
2018-11-17 12:55gitNote Added: 0081133
2018-11-17 21:58gitNote Added: 0081134
2018-11-17 22:56gitNote Added: 0081135
2018-11-17 23:17gitNote Added: 0081137
2018-11-18 08:02gitNote Added: 0081140
2018-11-18 08:03gitNote Added: 0081141
2018-11-20 08:31gitNote Added: 0081170
2018-11-20 14:22abvNote Added: 0081172
2018-11-20 14:22abvAssigned Toabv => msv
2018-11-20 14:22abvStatusnew => resolved
2018-11-20 14:22abvCategoryOCCT:Modeling Algorithms => OCCT:DRAW
2018-11-20 17:58msvNote Added: 0081181
2018-11-20 17:58msvAssigned Tomsv => abv
2018-11-20 17:58msvStatusresolved => assigned
2018-11-20 18:03msvNote Added: 0081182
2018-11-21 06:28gitNote Added: 0081186
2018-11-21 10:02abvNote Added: 0081187
2018-11-22 00:35msvNote Added: 0081203
2018-11-22 01:21msvNote Added: 0081205
2018-11-22 08:38abvNote Added: 0081206
2018-11-22 10:46msvNote Added: 0081207
2018-11-22 23:39abvNote Added: 0081222
2018-11-23 00:28abvNote Added: 0081223
2018-11-23 00:46gitNote Added: 0081224
2018-11-23 01:11gitNote Added: 0081225
2018-11-23 01:13gitNote Added: 0081226
2018-11-23 01:14abvNote Edited: 0081223bug_revision_view_page.php?bugnote_id=81223#r20419
2018-11-23 01:14abvRelationship addedrelated to 0024012
2018-11-23 07:49abvNote Added: 0081227
2018-11-23 07:49abvAssigned Toabv => msv
2018-11-23 07:49abvStatusassigned => resolved
2018-11-23 10:37msvNote Added: 0081228
2018-11-23 10:41msvNote Added: 0081229
2018-11-23 10:41msvAssigned Tomsv => bugmaster
2018-11-23 10:41msvStatusresolved => reviewed
2018-11-23 11:15apnNote Added: 0081232
2018-11-23 11:15apnStatusreviewed => tested
2018-11-25 13:40apnChangeset attached => occt master e05c25c1
2018-11-25 13:40apnAssigned Tobugmaster => apn
2018-11-25 13:40apnStatustested => verified
2018-11-25 13:40apnResolutionopen => fixed
2018-11-26 10:36gitNote Added: 0081254
2018-11-26 10:36gitNote Added: 0081255
2018-11-26 10:36gitNote Added: 0081259

Notes
(0069490)
apv   
2017-08-16 19:14   
Problem described in issue is reproduced on current state of OCCT.
(0081018)
abv   
2018-11-10 08:25   
There are two different problems actually:

1. OCCT problem

Method OSD_File::Capture() calls MSVCRT function _open_osfhandle() to get C file descriptor to the file opened using WinAPI function. This descriptor is created by that function and then never closed. This leads to overflow of the table of descriptors and the application gets terminated by C library (silent exit after definite number of DRAW commands captured by dlog).

2. Tcl problem

On Windows, standard channels (stdin, stdout, stderr) in Tcl are initialized by OS handles returned by WinAPI function GetStdHandle() (see TclpGetDefaultStdChannel() defined in win/tclWinChan.c).

These handles may be invalidated (closed, reopened, reassigned to different kind of object) by C/C++ code. In particular, function _dup2() of standard C library (MSVC), when called for the standard file number (0, 1, 2) as second argument, closes the OS handle associated with the standard stream, then creates the new handle and sets is as standard one by call to SetStdHandle().

In most cases the old and new handles are the same (apparently due to reuse), thus there are no immediate consequences. However, sometimes (in my experiments about once per several thousand calls), the new standard handle assigned by the system is different from the old one. Yet Tcl channel still keeps the old handle and when trying to use that channel (e.g. use puts to write to stdout), error occurs.

In this context, sometimes execution of a test script ends up with Tcl reporting
"error writing "stdout": bad file number"

For this problem, a ticket is created on Tcl:

https://core.tcl.tk/tcl/tktview/91c9bc1c457fda269ae18595944fc3c2b54d961d [^]

The proposed solution is to duplicate the standard handle returned by GetStdHandle() (in TclpGetDefaultStdChannel()) and to use the duplicate for initialization of the Tcl channel.

win/tclWinChan.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/win/tclWinChan.c b/win/tclWinChan.c
index 8c47be646..480b387f4 100644
--- a/win/tclWinChan.c
+++ b/win/tclWinChan.c
@@ -1302,6 +1302,17 @@ TclpGetDefaultStdChannel(
     return (Tcl_Channel) NULL;
     }

+ /*
+ * Make duplicate of the standard handle as it may be altered
+ * (closed, reopened with another type of the object etc.) by
+ * the system or a user code at any time, e.g. by call to _dup2()
+ */
+ if (! DuplicateHandle (GetCurrentProcess(), handle,
+ GetCurrentProcess(), &handle,
+ 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+ return (Tcl_Channel) NULL;
+ }
+
     channel = Tcl_MakeFileChannel(handle, mode);

    if (channel == NULL) {
(0081133)
git   
2018-11-17 12:55   
Branch CR27620 has been created by abv.

SHA-1: fcc99416e38c482cb2cf15c0f7069271a9ff45a4


Detailed log of new commits:

Author: abv
Date: Sat Nov 17 12:51:26 2018 +0300

    0027620: Test perf bop boxholes crashes DRAW
    
    1. Method OSD_File::Capture() is removed: on Windows it was allocating a C file descriptor for a file opened using WinAPI, and never released that descriptor (once allocated, it cannot be released separately from WinAPI file handle). Direct calls to dup/dup2 are used instead.
    
    2. In Draw_Window.cxx the standard Tcl channels are initialized manually using corrected version of Tcl internal function. This works around a problem with Tcl channels on Windows being bound to OS device handle owned by the system which can get invalidated as result of calls to dup2() (used to capture output to standard streams).
    
    3. Implementation of capturing of output to standard streams in DRAW (see command dlog) is revised:
    - Temporary file for capturing is opened once and used to store whole log, thus the need to collect log in the string stream in memory is avoided
    - Errors at capturing cout are handled and reported
    
    Test demo draw dlog is added
    Test demo draw getsource is corrected for VS2017 which generates file name in lowercase
(0081134)
git   
2018-11-17 21:58   
Branch CR27620 has been updated by abv.

SHA-1: 1154a01f52dc8e4b3ea850744733a58c149f31c6


Detailed log of new commits:

Author: abv
Date: Sat Nov 17 21:43:51 2018 +0300

    Fixes for Linux

(0081135)
git   
2018-11-17 22:56   
Branch CR27620 has been updated by abv.

SHA-1: 5d4ec0267c76ad2a8c245415aeaa7cf7c6465f37


Detailed log of new commits:

Author: abv
Date: Sat Nov 17 22:54:34 2018 +0300

    fix test

(0081137)
git   
2018-11-17 23:17   
Branch CR27620 has been updated forcibly by abv.

SHA-1: 18d7dc8e2321c428c26cd662c9e1495286bcc059
(0081140)
git   
2018-11-18 08:02   
Branch CR27620 has been updated forcibly by abv.

SHA-1: 1e56678a44de9f50d29203764f84772c18a119c6
(0081141)
git   
2018-11-18 08:03   
Branch CR27620 has been updated forcibly by abv.

SHA-1: 8860a60928875b2e52908a3f3426f97facbf70e0
(0081170)
git   
2018-11-20 08:31   
Branch CR27620 has been updated forcibly by abv.

SHA-1: fc1bce8cb783f18f7e7900e629cfeac102ddf461
(0081172)
abv   
2018-11-20 14:22   
The fix (resolving 0027620, 0027043, 0027013) is pushed to branch CR27620 and tested, see Jenkins job CR27620-master-abv. Please review
(0081181)
msv   
2018-11-20 17:58   
src/Draw/Draw_Interpretor.hxx
- rename "GetLogFileDescripror" to "GetLogFileDescriptor".
(0081182)
msv   
2018-11-20 18:03   
Why this difference presents on Linux?

IMAGE bugs modalg_5 bug24012: bug24012.png differs
(0081186)
git   
2018-11-21 06:28   
Branch CR27620_1 has been created by abv.

SHA-1: 56936afe23091cd10e42b952eae3c7588f6fa617


Detailed log of new commits:

Author: abv
Date: Sat Nov 17 12:51:26 2018 +0300

    0027620: Test perf bop boxholes crashes DRAW
    
    Implementation of capturing of output to standard streams in DRAW (see command dlog) is revised to avoid problems with command "test" executing long test scripts:
    
    1. Method OSD_File::Capture() is removed: on Windows it was allocating a C file descriptor for a file opened using WinAPI, and never released that descriptor (once allocated, it cannot be released separately from WinAPI file handle). Direct calls to dup/dup2 are used instead.
    
    2. In Draw_Window.cxx the standard Tcl channels are initialized manually using corrected version of Tcl internal function. This works around a problem with Tcl channels on Windows being bound to OS device handle owned by the system which can get invalidated as result of calls to dup2() (used to capture output to standard streams).
    
    3. Temporary file for capturing is opened once and used to store whole log, thus the need to collect log in the string stream in memory is avoided
    
    4. Possible errors of dup() and dup2() are checked and reported
    
    Test demo draw dlog is added
    Test demo draw getsource is corrected for VS2017 which generates file name in lowercase
(0081187)
abv   
2018-11-21 10:02   
Branch CR27620_1 contains renaming of GetLogFileDescripror. The tests are re-executed in the same Jenkins job. Alas, image for bug24012 is still empty on Debian. I have no idea why (the fix should not affect that test in any way), to be checked.
(0081203)
msv   
2018-11-22 00:35   
I have tried to change the script bug24012:
 vinit
-OCC24012 face edge
+nproject r edge face
 vsetdispmode 0
 vdisplay face
 vdisplay edge
+vdisplay r
+vsetcolor r yellow
 vfit


This solves the problem.
The changed script still reproduces the bug 0024012 in OCCT 6.9.1. So, I think the synthetic command OCC24012 is not needed at all.
(0081205)
msv   
2018-11-22 01:21   
In the command OCC24012, the result shape is displayed in the AIS viewer. If we suppress displaying the shape the problem is gone:

         Handle(AIS_InteractiveObject) myShape = new AIS_Shape (rshape);
         myAISContext->SetColor (myShape, Quantity_Color(Quantity_NOC_YELLOW), Standard_False);
-		myAISContext->Display (myShape, Standard_True);
+		//myAISContext->Display (myShape, Standard_True);


It is strange that this patch impacts displaying a shape from within a draw command.
The jenkins result is http://jenkins-test-12.nnov.opencascade.com/view/CR0-27620-master-MSV/view/TESTING/job/CR0-27620-master-MSV-OCCT-Debian80-64-opt-test-restart/HTML_20Report/ [^]
(0081206)
abv   
2018-11-22 08:38   
Thanks a lot Mikhail for your investigation! Yet have you been able to reproduce the problem on Linux (i.e. run test and see empty viewer)? On my side I have tried this on Ubuntu 16.04 with CLang and it worked without problem.

Test bugs modalg_5 bug24012 does not seem to check anything, IMHO it should be reworked to not only use nproject instead of specific DRAW command, but also to check the resulting shape. Do you agree with that?
(0081207)
msv   
2018-11-22 10:46   
I agree with you to revise the test bug24012.

I tested on Linux Debian 8 on current Jenkins-test-12 both my above changes. Both of them solve the problem with empty screen. And after reverting changes empty screen appeared again.

I think before closing this bug we should understand why this patch impacts behavior of the test when a shape is displayed from within the draw command OCC24012.
(0081222)
abv   
2018-11-22 23:39   
Valgrind report:

==9803== Conditional jump or move depends on uninitialised value(s)
==9803== at 0x1180AA45: BRepAlgo_NormalProjection::Build() (BRepAlgo_NormalProjection.cxx:474)
==9803== by 0x1A0FAC50: OCC24012(Draw_Interpretor&, int, char const**) (QABugs_19.cxx:1291)
...
(0081223)
abv   
2018-11-23 00:28   
(edited on: 2018-11-23 01:14)
The problem is in uninitialized field myFaceBounds. When it happens to be zero, the edge with bad BSpline curve is created (internal poles have oscillating coordinates with values as high as ~ 1e14). When that curve is displayed in 3d viewer, it shows nothing after fit all.

To reproduce this in stable way, add option "-g" to nproject command:

nproject r edge face -g

(0081224)
git   
2018-11-23 00:46   
Branch CR27620_1 has been updated by abv.

SHA-1: 0035fe272e3533d381eafd81e1a47773ca87509a


Detailed log of new commits:

Author: abv
Date: Fri Nov 23 00:43:37 2018 +0300

    Field myFaceBounds is initialized in constructor of the class BRepAlgo_NormalProjection to avoid undefined behavior; test bugs modalg_5 bug24012 is corrected to use command nproject instead of custom one, and to check propertes of the resulting shape

(0081225)
git   
2018-11-23 01:11   
Branch CR27620_1 has been updated by abv.

SHA-1: 4a1035ff665d61d774aa0d568895e6a48422304e


Detailed log of new commits:

Author: abv
Date: Fri Nov 23 01:08:32 2018 +0300

    Fix gcc warnings

(0081226)
git   
2018-11-23 01:13   
Branch CR27620_2 has been created by abv.

SHA-1: e0bca819be568dd17894709a7d1b768fc1bc78a1


Detailed log of new commits:

Author: abv
Date: Sat Nov 17 12:51:26 2018 +0300

    0027620: Test perf bop boxholes crashes DRAW
    
    Implementation of capturing of output to standard streams in DRAW (see command dlog) is revised to avoid problems with command "test" executing long test scripts:
    
    1. Method OSD_File::Capture() is removed: on Windows it was allocating a C file descriptor for a file opened using WinAPI, and never released that descriptor (once allocated, it cannot be released separately from WinAPI file handle). Direct calls to dup/dup2 are used instead.
    
    2. In Draw_Window.cxx the standard Tcl channels are initialized manually using corrected version of Tcl internal function. This works around a problem with Tcl channels on Windows being bound to OS device handle owned by the system which can get invalidated as result of calls to dup2() (used to capture output to standard streams).
    
    3. Temporary file for capturing is opened once and used to store whole log, thus the need to collect log in the string stream in memory is avoided
    
    4. Possible errors of dup() and dup2() are checked and reported
    
    Test demo draw dlog is added
    
    Off-topic changes:
    
    - Test demo draw getsource is corrected for VS2017 which generates file name in lowercase
    - Field myFaceBounds is initialized in constructor of the class BRepAlgo_NormalProjection to avoid undefined behavior
    - Test bugs modalg_5 bug24012 is corrected to use command nproject instead of custom one, and to check propertes of the resulting shape
(0081227)
abv   
2018-11-23 07:49   
Please review corrected branch CR27620, the same Jenkins job.

Regarding 0024012, please decide on whether it should be reopened. In my understanding, the problem reported in that issue (bad curve) has never been solved. Yet the reproducer was depending on uninitialized field, and over time the code drifted to the state that it did not trigger wrong behavior in the tests. Still the originally reported erroneous behavior can be reproduced by adding -g to nproject command in the test script.
(0081228)
msv   
2018-11-23 10:37   
I think the issue 0024012 should not be reopened. The option SetLimit() should be used in such cases. If the caller is confident that the projection fits in surface boundaries he can save time by disabling boundaries checking.
(0081229)
msv   
2018-11-23 10:41   
Reviewed.
(0081232)
apn   
2018-11-23 11:15   
Combination -
OCCT branch : CR27620_2 SHA - e0bca819be568dd17894709a7d1b768fc1bc78a1
Products branch : master SHA - dc9b54ff96bfdd5be36ff17bb512772a70f930db
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 16316.060000000056 / 16331.970000000008 [-0.10%]
Products
Total CPU difference: 7041.690000000034 / 7059.160000000035 [-0.25%]
Windows-64-VC14:
OCCT
Total CPU difference: 17779.1875 / 17739.59375 [+0.22%]
Products
Total CPU difference: 8513.953125 / 8542.921875 [-0.34%]

Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0081254)
git   
2018-11-26 10:36   
Branch CR27620_2 has been deleted by inv.

SHA-1: e0bca819be568dd17894709a7d1b768fc1bc78a1
(0081255)
git   
2018-11-26 10:36   
Branch CR27620_1 has been deleted by inv.

SHA-1: 4a1035ff665d61d774aa0d568895e6a48422304e
(0081259)
git   
2018-11-26 10:36   
Branch CR27620 has been deleted by inv.

SHA-1: fc1bce8cb783f18f7e7900e629cfeac102ddf461