Project

General

Profile

Bug #11945

Potential deadlock in frame_generator_pre when running multi-threaded conversion

Added by Alexandru Lungu 7 days ago. Updated 6 days ago.

Status:
Test
Priority:
Normal
Assignee:
Octavian Adrian Gavril
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
16789
reviewer:
Alexandru Lungu
production:
No
env_name:
topics:
Conversion

History

#1 Updated by Alexandru Lungu 7 days ago

  • Assignee set to Octavian Adrian Gavril

I hit a case in conversion when two threads were racing on the same TRPL code. This made both stuck. In rules/annotations/frame_generator_pre.xml:

  <variable name="mockFormAst" type="com.goldencode.ast.Aast" multiThreadAccess="true" />   <!-- line 118, GLOBAL
  -->
  ...
  <while>iter.hasNext()
     <action>ref = iter.next()</action>
     <action>mockFormAst = EMPTY_FORM_AST.duplicateFresh()</action>
     <action>mockFormAst.setHidden(true)</action>
     <action>copy.graft(mockFormAst)</action>
     <action>execLib("processForm", mockFormAst, ...)</action>

1. Thread A sets mockFormAst = X and grafts X.
2. Thread B sets mockFormAst = Y before A's next iteration.
3. A's next graft also uses Y, and so does B's. Y ends up in two trees, or grafted twice into the same parent. The second graft makes the current tail, Y, point to Y as its next sibling, so the chain loops back on itself.
4. The next addChild on that parent spins forever. Both trees are corrupted, which is why both threads hang at the same spot.

#2 Updated by Octavian Adrian Gavril 7 days ago

I'm looking into this right away.

#3 Updated by Octavian Adrian Gavril 7 days ago

I investigated the area, and it's pretty clear that mockFormAst should not be shared across threads. It shouldn't be declared as a global variable either, since it's only used within that <while> loop. I might have marked it that way by mistake when I flagged EMPTY_FORM_AST.

I think I just ran into this blocker while converting a module app for #10566.

I'm creating a branch and committing the fix right now.

#4 Updated by Octavian Adrian Gavril 7 days ago

  • Status changed from New to WIP
  • % Done changed from 0 to 100
  • reviewer Alexandru Lungu added

Created branch 11945a.

I moved the mockFormAst declaration from global to rule-set scope. This is committed in rev16788.

Alex, please review the change.

#5 Updated by Octavian Adrian Gavril 7 days ago

  • Status changed from WIP to Review

#6 Updated by Alexandru Lungu 7 days ago

  • Status changed from Review to Internal Test

I am OK with the changes. It also fixes the conversion in question.
Feel free to smoke test some conversions. When ready, please let me know so I can grant the green light for merging.

#7 Updated by Octavian Adrian Gavril 7 days ago

Alexandru Lungu wrote:

I am OK with the changes. It also fixes the conversion in question.
Feel free to smoke test some conversions. When ready, please let me know so I can grant the green light for merging.

I completed full conversion regression testing for Hotel GUI, Hotel ChUI, ChUI regression testing app, and the OO application. All conversions completed successfully, with no changes in the generated output.

Is any additional testing required?

#8 Updated by Alexandru Lungu 6 days ago

  • Status changed from Internal Test to Merge Pending

No, please merge to trunk now.

#9 Updated by Octavian Adrian Gavril 6 days ago

  • Status changed from Merge Pending to Test
  • version_resolved set to 16789
  • topics Conversion added

Branch 11945a was merged into trunk as rev. 16789 and archived.

Also available in: Atom PDF