View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0027620Open CASCADEOCCT:DRAWpublic2016-06-20 11:132018-11-26 10:36
ReporterabvAssigned Toapn  
PrioritynormalSeveritymajor 
Status closedResolutionfixed 
Target Version7.4.0Fixed in Version7.4.0 
Summary0027620: Test perf bop boxholes crashes DRAW
DescriptionWhen 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.
Steps To ReproduceProblem 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 }
TagsNo tags attached.
Test case numberperf bop boxholes

Relationships

related to 0027043 closedbugmaster Open CASCADE Draw is sometimes crashed unexpectedly during running of test command 
related to 0024012 closedifv Community problem with BRepAlgo_NormalProjection 

Activities

apv

2017-08-16 19:14

tester   ~0069490

Problem described in issue is reproduced on current state of OCCT.

abv

2018-11-10 08:25

manager   ~0081018

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) {

git

2018-11-17 12:55

administrator   ~0081133

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

git

2018-11-17 21:58

administrator   ~0081134

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

git

2018-11-17 22:56

administrator   ~0081135

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

git

2018-11-17 23:17

administrator   ~0081137

Branch CR27620 has been updated forcibly by abv.

SHA-1: 18d7dc8e2321c428c26cd662c9e1495286bcc059

git

2018-11-18 08:02

administrator   ~0081140

Branch CR27620 has been updated forcibly by abv.

SHA-1: 1e56678a44de9f50d29203764f84772c18a119c6

git

2018-11-18 08:03

administrator   ~0081141

Branch CR27620 has been updated forcibly by abv.

SHA-1: 8860a60928875b2e52908a3f3426f97facbf70e0

git

2018-11-20 08:31

administrator   ~0081170

Branch CR27620 has been updated forcibly by abv.

SHA-1: fc1bce8cb783f18f7e7900e629cfeac102ddf461

abv

2018-11-20 14:22

manager   ~0081172

The fix (resolving 0027620, 0027043, 0027013) is pushed to branch CR27620 and tested, see Jenkins job CR27620-master-abv. Please review

msv

2018-11-20 17:58

developer   ~0081181

src/Draw/Draw_Interpretor.hxx
- rename "GetLogFileDescripror" to "GetLogFileDescriptor".

msv

2018-11-20 18:03

developer   ~0081182

Why this difference presents on Linux?

IMAGE bugs modalg_5 bug24012: bug24012.png differs

git

2018-11-21 06:28

administrator   ~0081186

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

abv

2018-11-21 10:02

manager   ~0081187

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.

msv

2018-11-22 00:35

developer   ~0081203

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.

msv

2018-11-22 01:21

developer   ~0081205

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/

abv

2018-11-22 08:38

manager   ~0081206

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?

msv

2018-11-22 10:46

developer   ~0081207

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.

abv

2018-11-22 23:39

manager   ~0081222

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)
...

abv

2018-11-23 00:28

manager   ~0081223

Last edited: 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

git

2018-11-23 00:46

administrator   ~0081224

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

git

2018-11-23 01:11

administrator   ~0081225

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

git

2018-11-23 01:13

administrator   ~0081226

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

abv

2018-11-23 07:49

manager   ~0081227

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.

msv

2018-11-23 10:37

developer   ~0081228

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.

msv

2018-11-23 10:41

developer   ~0081229

Reviewed.

apn

2018-11-23 11:15

administrator   ~0081232

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

git

2018-11-26 10:36

administrator   ~0081254

Branch CR27620_2 has been deleted by inv.

SHA-1: e0bca819be568dd17894709a7d1b768fc1bc78a1

git

2018-11-26 10:36

administrator   ~0081255

Branch CR27620_1 has been deleted by inv.

SHA-1: 4a1035ff665d61d774aa0d568895e6a48422304e

git

2018-11-26 10:36

administrator   ~0081259

Branch CR27620 has been deleted by inv.

SHA-1: fc1bce8cb783f18f7e7900e629cfeac102ddf461

Related Changesets

occt: master e05c25c1

2018-11-17 09:51:26

abv


Committer: apn Details Diff		 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		 Affected Issues
0027620
mod - src/BRepAlgo/BRepAlgo_NormalProjection.cxx Diff File
mod - src/Draw/Draw_BasicCommands.cxx Diff File
mod - src/Draw/Draw_Interpretor.cxx Diff File
mod - src/Draw/Draw_Interpretor.hxx Diff File
mod - src/Draw/Draw_Window.cxx Diff File
mod - src/OSD/OSD_File.cxx Diff File
mod - src/OSD/OSD_File.hxx Diff File
mod - src/QABugs/QABugs_19.cxx Diff File
mod - tests/bugs/modalg_5/bug24012 Diff File
add - tests/demo/draw/dlog Diff File
mod - tests/demo/draw/getsource Diff File

Issue History

Date Modified Username Field Change
2016-06-20 11:13 abv New Issue
2016-06-20 11:13 abv Assigned To => msv
2016-06-20 15:27 msv Relationship added related to 0027043
2016-06-20 15:35 abv Assigned To msv => abv
2016-10-28 16:28 msv Target Version 7.1.0 => 7.2.0
2017-07-24 09:32 msv Target Version 7.2.0 => 7.3.0
2017-08-16 19:13 apv Test case number => perf bop boxholes
2017-08-16 19:14 apv Note Added: 0069490
2017-12-05 16:59 msv Target Version 7.3.0 => 7.4.0
2018-11-10 08:25 abv Note Added: 0081018
2018-11-11 21:12 abv Steps to Reproduce Updated
2018-11-17 12:55 git Note Added: 0081133
2018-11-17 21:58 git Note Added: 0081134
2018-11-17 22:56 git Note Added: 0081135
2018-11-17 23:17 git Note Added: 0081137
2018-11-18 08:02 git Note Added: 0081140
2018-11-18 08:03 git Note Added: 0081141
2018-11-20 08:31 git Note Added: 0081170
2018-11-20 14:22 abv Note Added: 0081172
2018-11-20 14:22 abv Assigned To abv => msv
2018-11-20 14:22 abv Status new => resolved
2018-11-20 14:22 abv Category OCCT:Modeling Algorithms => OCCT:DRAW
2018-11-20 17:58 msv Note Added: 0081181
2018-11-20 17:58 msv Assigned To msv => abv
2018-11-20 17:58 msv Status resolved => assigned
2018-11-20 18:03 msv Note Added: 0081182
2018-11-21 06:28 git Note Added: 0081186
2018-11-21 10:02 abv Note Added: 0081187
2018-11-22 00:35 msv Note Added: 0081203
2018-11-22 01:21 msv Note Added: 0081205
2018-11-22 08:38 abv Note Added: 0081206
2018-11-22 10:46 msv Note Added: 0081207
2018-11-22 23:39 abv Note Added: 0081222
2018-11-23 00:28 abv Note Added: 0081223
2018-11-23 00:46 git Note Added: 0081224
2018-11-23 01:11 git Note Added: 0081225
2018-11-23 01:13 git Note Added: 0081226
2018-11-23 01:14 abv Note Edited: 0081223
2018-11-23 01:14 abv Relationship added related to 0024012
2018-11-23 07:49 abv Note Added: 0081227
2018-11-23 07:49 abv Assigned To abv => msv
2018-11-23 07:49 abv Status assigned => resolved
2018-11-23 10:37 msv Note Added: 0081228
2018-11-23 10:41 msv Note Added: 0081229
2018-11-23 10:41 msv Assigned To msv => bugmaster
2018-11-23 10:41 msv Status resolved => reviewed
2018-11-23 11:15 apn Note Added: 0081232
2018-11-23 11:15 apn Status reviewed => tested
2018-11-25 13:40 apn Changeset attached => occt master e05c25c1
2018-11-25 13:40 apn Assigned To bugmaster => apn
2018-11-25 13:40 apn Status tested => verified
2018-11-25 13:40 apn Resolution open => fixed
2018-11-26 10:36 git Note Added: 0081254
2018-11-26 10:36 git Note Added: 0081255
2018-11-26 10:36 git Note Added: 0081259