On Fri, Sep 29, 2017 at 9:03 AM, Thomas Koenig <tkoe...@netcologne.de> wrote: > I wrote: > >> This gets rid of the thread sanitizer issue; the thread sanitizer >> output is clean. However, I would appreciate feedback about whether >> this approach (and my code) is correct. >> >> Regression-tested. >> > > > Here is an update: The previous version of the patch had a regression, > which is removed (with a test case) with the current version. > Also regression-tested. > > So, > >> Comments? Suggestions for improvements/other approaches? Close the PR >> as WONTFIX instead? OK for trunk?
In general, I do think this is an issue that should be fixed. Multithreading is only going get more pervasive, and gfortran spewing false positives from tools such as threadsanitizer is definitely unhelpful. Now, for the patch itself, AFAICT the general approach is correct. However, I have a couple of questions and/or suggestions for improvements: 1) I'm confused why fbuf_destroy is modified. The fbuf structure is part of gfc_unit, and should be accessed with the same locking rules as the rest of the gfc_unit components. When closing the unit, I think the same should apply here, no? 2) I think the mutex init stuff can remain in insert_unit, just the locking needs to move out and below freeing unit_lock like you have done. -- Janne Blomqvist
