FDS Source: Issue #14387. Fix OpenMP race condition

[mcgratta]

No description provided.

[rmcdermo]

@mcgratta Can you please explain. Is this to tell us to not use NULL() pointers anymore?

[mcgratta]

Just a comment -- Marcos just discovered that doing this

TYPE(LAGRANGIAN_PARTICLE_CLASS_TYPE), POINTER :: LPC=>NULL()

caused an OpenMP data race. I did not realize this, but apparently when a pointer is initialized to null, as here, it automatically assumes a SAVE attribute. The subroutine containing this line was called by two OpenMP threads such that LPC pointed to the same location in memory. I was told by the thread sanitizer that this variable was global, which made no sense to me until we discovered this SAVE feature, meaning that this pointer is now saved outside the routine.

Long story short, I'm not sure we need to nullify pointers upon declaration. I can't remember when and why we started this. Does anyone remember?

[mcgratta]

I don't normally initialize by pointing to NULL(). The question is whether we should remove existing pointers or let them be. Pointers are only 8 bytes, so I don't think they add to the stack or heap or whatever. My only concern is whether they will inadvertently cause data races if we have multiple threads calling the same subroutine and pointing at the same thing.

[mcgratta]

BTW, if we accidently use a pointer before it is associated, the -check all debug option will flag it at run time. It is often a trick I use to make sure that I've set the pointers appropriately.

[marcosvanella]

Just a comment -- Marcos just discovered that doing this

TYPE(LAGRANGIAN_PARTICLE_CLASS_TYPE), POINTER :: LPC=>NULL()

caused an OpenMP data race. I did not realize this, but apparently when a pointer is initialized to null, as here, it automatically assumes a SAVE attribute. The subroutine containing this line was called by two OpenMP threads such that LPC pointed to the same location in memory. I was told by the thread sanitizer that this variable was global, which made no sense to me until we discovered this SAVE feature, meaning that this pointer is now saved outside the routine.

Long story short, I'm not sure we need to nullify pointers upon declaration. I can't remember when and why we started this. Does anyone remember?

This is something we found out with Chandan while debugging chemistry. We don't want to have initialization and declaration of pointers in subroutines that get called inside an OpenMP loop, as they will be shared between threads leading to data races.

[mcgratta]

I removed all the pointer initializations to NULL() throughout the code.