Looks good Tobias, OK,

Odd about that line in set_internal_unit. Probably leftover from a copy/paste. I see in comment #5 of the PR that you mentioned trying the assert to make sure it is useless code.

Thanks for the patch,

Jerry

On 3/11/21 2:38 AM, Tobias Burnus wrote:
Revised version – the previous had a lock inversion, which could lead to a deadlock, found by -fsanitize=thread. See transfer.c for the changes.

OK?

Tobias

On 11.03.21 10:42, Tobias Burnus wrote:
Hi all,

as found by Martin (thanks!) there is a race for newunit_free.
While that call is within the unitlock for the calls in io/unit.c,
the call in transfer.c did not use locks.

Additionally,
      unit = get_gfc_unit (dtp->common.unit, do_create);
      set_internal_unit (dtp, unit, kind);
gets first the unit (with proper locking when using the unit number
dtp->common.unit) but then in set_internal_unit it re-sets the
unit number to the same number without locking. That causes
race warnings and if the assignment is not atomic it is a true race.

OK for mainline? What about GCC 10?

As Martin notes in the email thread and in the PR there are more
race warnings (and likely true race issues).

Tobias


Reply via email to