Prepartitioning with threading race condition fix - #450
Conversation
mo-marqh
left a comment
There was a problem hiding this comment.
A change is necessary to fix the observed edge case bug.
This is a neat and well contained alternative that meets requirements
|
ready for code review Benjamin Went (@MetBenjaminWent) |
| type(linked_list_item_type),pointer :: loop => null() | ||
| type(linked_list_item_type),pointer :: loop | ||
|
|
||
| nullify(loop) |
There was a problem hiding this comment.
Should this instead be, (for consistency)? : loop => null()
There was a problem hiding this comment.
Thanks Benjamin Went (@MetBenjaminWent), agreed, have pushed this change now.
Ricky Wong (mo-rickywong)
left a comment
There was a problem hiding this comment.
No issue with this, It's actually recommended practice which we try to remove null() assignments on the declaration statement. This is obviously legacy code that hasn't been revisited.
Benjamin Went (MetBenjaminWent)
left a comment
There was a problem hiding this comment.
Thanks for the changes.
Developer testing all passes, partitioning runs as expected still with lfric apps as well, and with the latest offline partitioning updates.
CR Approved
3e965e8
into
MetOffice:main
Co-authored-by: Benjamin Went <136574563+MetBenjaminWent@users.noreply.github.com>
PR Summary
Sci/Tech Reviewer: mo-marqh
Code Reviewer: Benjamin Went (@MetBenjaminWent)
This PR is to address a race condition in the threading employed by the partitioning section of the
cubedsphere_mesh_generatorcode. This was originally found using offline partitioning in the apps PR #688 (linked to this one) where a seg fault was occurring for really high partition numbers and a custom mesh decomposition task. I believe that the linked list pointer in theglobal_mesh_map_collectioncode was being overwritten by competing threads and that by separating the declaration and nullification of the pointer, thesaveattribute is no longer being applied, making theloopvariable local to each thread calling the object.Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_core - partitioned_null_pointer_dev_tests/run1
Suite Information
Task Information
✅ succeeded tasks - 431
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review