MantisBT - Open CASCADE
View Issue Details
0031785Open CASCADE[OCCT] OCCT:Application Frameworkpublic2020-09-21 22:142020-10-03 14:06
kgv 
bugmaster 
highblock 
verifiedfixed 
[OCCT] 7.5.0 
[OCCT] 7.5.0 
bugs/caf/bug31785
0031785: [REGRESSION] Application Framework - application crashes on reading XBF document in background thread
Existing multi-threaded application crashes due to NULL dereference after porting to 0029195.
Apparently - either porting notes are missing, or proposed patch is broken.

PCDM_ReaderStatus CDF_Application::CanRetrieve() {
...
    Handle(CDM_MetaData) theMetaData = myMetaDataDriver->MetaData(aFolder,aName,aVersion);

    if(theMetaData->IsRetrieved()) { // << theMetaData is NULL here
bugs caf bug31785
No tags attached.
has duplicate 0031807closed bugmaster Community TDocStd_Application::Open (const TCollection_ExtendedString&,...) crash in non-main thread 
child of 0029195verified abv Open CASCADE OCAF - ensure thread safety for different documents 
Issue History
2020-09-21 22:14kgvNew Issue
2020-09-21 22:14kgvAssigned To => mpv
2020-09-21 22:14kgvRelationship addedchild of 0029195
2020-09-21 23:42kgvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=23556#r23556
2020-09-22 09:39kgvSummaryApplication Framework - application crashes after porting to 0029195 => [REGRESSION] Application Framework - application crashes on reading XBF document in background thread
2020-09-22 09:39kgvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=23559#r23559
2020-09-22 09:39kgvPrioritynormal => high
2020-09-22 15:22gitNote Added: 0095204
2020-09-22 15:23mpvNote Added: 0095205
2020-09-22 15:23mpvStatusnew => resolved
2020-09-22 15:23mpvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=23565#r23565
2020-09-22 15:24mpvAssigned Tompv => szy
2020-09-22 15:46kgvNote Added: 0095206
2020-09-22 15:46kgvAssigned Toszy => mpv
2020-09-22 15:46kgvStatusresolved => assigned
2020-09-22 15:47kgvNote Edited: 0095206bug_revision_view_page.php?bugnote_id=95206#r23567
2020-09-23 12:01gitNote Added: 0095229
2020-09-23 12:09gitNote Added: 0095230
2020-09-23 12:37gitNote Added: 0095232
2020-09-23 15:55mpvNote Added: 0095241
2020-09-23 15:55mpvAssigned Tompv => kgv
2020-09-23 15:55mpvStatusassigned => resolved
2020-09-23 15:55mpvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=23577#r23577
2020-09-23 16:05kgvNote Added: 0095242
2020-09-23 16:05kgvAssigned Tokgv => mpv
2020-09-23 16:05kgvStatusresolved => assigned
2020-09-23 16:06kgvNote Edited: 0095242bug_revision_view_page.php?bugnote_id=95242#r23579
2020-09-23 18:10abvAssigned Tompv => abv
2020-09-23 18:12abvNote Added: 0095248
2020-09-23 18:54mpvNote Added: 0095251
2020-09-24 15:18gitNote Added: 0095269
2020-09-24 18:18gitNote Added: 0095282
2020-09-24 21:53gitNote Added: 0095293
2020-09-25 07:31abvNote Added: 0095298
2020-09-25 07:31abvAssigned Toabv => kgv
2020-09-25 07:31abvStatusassigned => resolved
2020-09-25 09:53gitNote Added: 0095300
2020-09-25 09:54gitNote Added: 0095301
2020-09-25 09:57kgvNote Added: 0095302
2020-09-25 09:57kgvAssigned Tokgv => bugmaster
2020-09-25 09:57kgvStatusresolved => reviewed
2020-09-25 10:07gitNote Added: 0095303
2020-09-25 10:07abvNote Added: 0095304
2020-09-27 12:16bugmasterTest case number => bugs/caf/bug31785
2020-09-27 14:12bugmasterNote Added: 0095368
2020-09-27 14:12bugmasterStatusreviewed => tested
2020-09-27 14:13bugmasterChangeset attached => occt master 0515f5b3
2020-09-27 14:13bugmasterStatustested => verified
2020-09-27 14:13bugmasterResolutionopen => fixed
2020-09-27 14:29gitNote Added: 0095378
2020-09-27 14:29gitNote Added: 0095381
2020-09-30 12:36mpvRelationship addedhas duplicate 0031807
2020-10-03 14:05bugmasterChangeset attached => occt master 8a39adb7

Notes
(0095204)
git   
2020-09-22 15:22   
Branch CR31785 has been created by mpv.

SHA-1: e2bbb9d2039e572bf09cd6cc8d5bbc7f64324cbe


Detailed log of new commits:

Author: mpv
Date: Tue Sep 22 15:24:54 2020 +0300

    0031785: [REGRESSION] Application Framework - application crashes on reading XBF document in background thread
    
    The porting notes are added.
(0095205)
mpv   
2020-09-22 15:23   
Updated the upgrade.md porting notes file. No tests needed.
(0095206)
kgv   
2020-09-22 15:46   
(edited on: 2020-09-22 15:47)
- Please integrate the test case with fixed behavior.
- Please add assertion checks / error messages indicating API misuse instead of NULL dereference crashes.
- Please provide more detailed description in Upgrade Guide on how applications following common conception of multi-thread safety (e.g. accessing documents from multiple threads using mutex locks) should be ported (preferably with code sample.)

(0095229)
git   
2020-09-23 12:01   
Branch CR31785 has been updated by mpv.

SHA-1: 02207811df1727f57bc21ccded998b4d8f4f3abc


Detailed log of new commits:

Author: mpv
Date: Wed Sep 23 12:04:20 2020 +0300

    # taking into account remarks

(0095230)
git   
2020-09-23 12:09   
Branch CR31785 has been updated by mpv.

SHA-1: 2bab77bbe80456b66a5b7a4365bd156cc8b5831e


Detailed log of new commits:

Author: mpv
Date: Wed Sep 23 12:12:33 2020 +0300

    # taking into account remarks

(0095232)
git   
2020-09-23 12:37   
Branch CR31785 has been updated by mpv.

SHA-1: 874a38ef63aef0ff7c7ff198a5a20a5502b1e3a5


Detailed log of new commits:

Author: mpv
Date: Wed Sep 23 12:40:01 2020 +0300

    # get rid of warnings

(0095241)
mpv   
2020-09-23 15:55   
Resolved. Remarks are fixed.

Tests with new case are passed:
http://jenkins-test-12.nnov.opencascade.com/view/CR31785-master-MPV/view/COMPARE/ [^]

CR31785-master-MPV-OCCT-Debian80-64-opt-test-compare is failed in "perf ncollection A1", which was not touched by this fix.
(0095242)
kgv   
2020-09-23 16:05   
(edited on: 2020-09-23 16:06)
+In that way, do not create descedants of the *CDF_Application* in one thread and pass it into
+another thread as argument or reuse it in different threads through global variables definition.
+Instead, create a new application instance in each thread.

This does not clarify how existing multi-threaded applications are expected to be ported.
How existing algorithms are expected to be modified?
If creating an Application is a mandatory demand by new API, then CDF_Application should provide a virtual interface "Clone()" implemented in all subclasses, with which algorithm will be able to copy existing application from one thread to another.

Such design limitation looks awkward to me anyway and contradicts to general understanding on how normal objects are expected to be used in multithreaded environment.

+      Message::SendFail() << "Error: no exception produced when accessing application created 
in another thread";

Please also make a test opening document with success in background thread.

(0095248)
abv   
2020-09-23 18:12   
Colleagues, I am going to work on this issue to see if "same-thread" limitation is relevant at all, and try to get rid of it
(0095251)
mpv   
2020-09-23 18:54   
> Please also make a test opening document with success in background thread.

This is already done with multiple parallel threads in test "bugs caf bug29195_1"
(0095269)
git   
2020-09-24 15:18   
Branch CR31785 has been updated by abv.

SHA-1: 9e48eda1515abf1f152ed7a8ba948a6ec0012ef2


Detailed log of new commits:

Author: abv
Date: Thu Sep 24 15:21:25 2020 +0300

    CDF_Session is removed, LookUp tables are localized in application instances

(0095282)
git   
2020-09-24 18:18   
Branch CR31785 has been updated by abv.

SHA-1: 452cde09a3a9a767afdfe1bf2850e40e1e92776e


Detailed log of new commits:

Author: abv
Date: Thu Sep 24 18:21:40 2020 +0300

    Fix Open()

(0095293)
git   
2020-09-24 21:53   
Branch CR31785_1 has been created by abv.

SHA-1: 9f95d07a27be3816835831c2e760c5707a1a55dd


Detailed log of new commits:

Author: mpv
Date: Tue Sep 22 15:24:54 2020 +0300

    0031785: [REGRESSION] Application Framework - application crashes on reading XBF document in background thread
    
    Class CDF_Session is removed.
    
    Integrated previously but not described:
    
    0029195: OCAF - ensure thread safety for different documents.
    
    Static local variables are eliminated in PCDM package.
    Global documents metadata look-up table and directory of opened documents are removed.
    Look-up table is maintained now as field in instances of the CDM_Application class.
    Methods providing iteration by components are removed from class CDF_Store; signature of methods returned Standard_ExtString is changed to return Handle(TCollection_HExtendedString).
    Support of different "Presentations" of documents is eliminated.
(0095298)
abv   
2020-09-25 07:31   
Updated fix is pushed to CR31785_1 and tested, see Jenkins job CR31785-master-MPV. Note that one test failed, this is apparently false positive (small memory leak) - this test is OK on restart. Please review
(0095300)
git   
2020-09-25 09:53   
Branch CR31785_1 has been updated forcibly by kgv.

SHA-1: 7fb84e6a022107447d7a9387eb7ccb6b6db99d14
(0095301)
git   
2020-09-25 09:54   
Branch CR31785_1 has been updated forcibly by kgv.

SHA-1: c037161758b57174d6764e64187ab63f42ab03a5
(0095302)
kgv   
2020-09-25 09:57   
Please raise the patch
- OCCT branch: CR31785_1.
(0095303)
git   
2020-09-25 10:07   
Branch CR31785_1 has been updated forcibly by abv.

SHA-1: 888c4c041ebe5f55cbd55d49a26aec40f52454b6
(0095304)
abv   
2020-09-25 10:07   
Sorry for one more forced push - that was to remove unsed file, not affecting anything else
(0095368)
bugmaster   
2020-09-27 14:12   
Combination -
OCCT branch : IR-2020-09-25
master SHA - d7bc5c833ec064bd103ebbff2882146ad5a7e7de
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : IR-2020-09-25 SHA - a8c0c30ba368a2503bbdf9800228ace93993dfff
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: 18039.260000000093 / 17637.5100000001 [+2.28%]
Products
Total CPU difference: 12124.200000000114 / 12092.06000000011 [+0.27%]
Windows-64-VC14:
OCCT
Total CPU difference: 19753.9375 / 18982.296875 [+4.07%]
Products
Total CPU difference: 13527.921875 / 13315.40625 [+1.60%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0095378)
git   
2020-09-27 14:29   
Branch CR31785_1 has been deleted by inv.

SHA-1: 888c4c041ebe5f55cbd55d49a26aec40f52454b6
(0095381)
git   
2020-09-27 14:29   
Branch CR31785 has been deleted by inv.

SHA-1: 452cde09a3a9a767afdfe1bf2850e40e1e92776e