clarification: unlock of freed lock

[Former user]

Originally reported on Google Code with ID 49 ``` Section 7.2.4.4 of the 1.2 spec says, in part:

3 upc_lock_free succeeds regardless of whether the referenced lock is currently unlocked or currently locked (by any thread).

4 Any subsequent calls to locking functions from any thread using ptr have undefined effects. This also applies to any thread currently calling upc_lock.

This means the following is OK: upc_lock(ptr); upc_lock_free(prt);

However, it is unclear how to interpret "locking functions" in (4). One interpretation hinges on the use of the work "lockING" and includes ONLY upc_lock() and upc_lock_attempt(), which makes the following OK: upc_lock(ptr); upc_lock_free(prt); upc_unlock(ptr);

While a stricter interpretation would include upc_unlock() among the "locking functions", and make the upc_unlock() call above illegal.

The first interpretation (that ALLOWS unlock-after-free) is problematic because it may require deferring at least some of the freeing of resources until the unlock call, WHICH MAY NEVER HAPPEN (especially if the user has read the spec as having the second interpretation).

Under the second (disallow-unlock-after-free) one can destroy the lock completely in upc_lock_free().

Proposal:

In 7.2.4.4(4) replace:

"Any subsequent calls to locking functions from any thread using ptr have undefined effects."

with

"Subsequent calls from any thread to functions defined in Section 7.2.4, using ptr, have undefined behavior."

Bonus proposal:

It might also be good to replace "This also applies to any thread currently calling upc_lock." with "This also applies to any call to upc_lock which is blocked at the time of the call to upc_lock_free". Or similar (to goal here is to apply to a CALL not a THREAD).

```

Reported by `[e-mail]` on 2012-06-11 20:07:43

[Former user]

``` I authored the original text in question, and the intent was to disallow unlock (or any other calls) on a lock after free (or concurrent with free). I agree the language should be tightened up to avoid the unintentional misinterpretation.

I agree with the proposed clarification, although it occurs to me that the second sentence may be confusing because the case it covers is not technically a "subsequent call". I'd also like to clarify the restriction on ptr applies to the referenced lock object, not the pointer used to name it.

Improved Proposal:

In 7.2.4.4(4) replace:

"Any subsequent calls to locking functions from any thread using ptr have undefined effects."

with

"Subsequent or concurrent calls from any thread to functions defined in Section 7.2.4 using the lock pointed by ptr have undefined behavior. Footnote: This also applies to any call to upc_lock on the the lock pointed by ptr which is blocked at the time of the call to upc_lock_free."

```

Reported by `danbonachea` on 2012-06-15 06:55:08

[Former user]

``` typox2: "pointed TO by ptr" ```

Reported by `danbonachea` on 2012-06-15 07:07:05

[Former user]

``` I agree that Dan's proposal (with the typo fixed) is superior to the one I offered initially. ```

Reported by `[e-mail]` on 2012-06-15 07:20:19

[Former user]

Reported by `gary.funck` on 2012-07-03 15:07:49 - Labels added: Type-Clarification - Labels removed: Type-Defect

[Former user]

``` Here is the exact change proposal:

--- upc-lib-core.tex (revision 80) +++ upc-lib-core.tex (working copy) @@ -403,9 +379,10 @@ referenced lock is currently unlocked or currently locked (by any thread).

-\np Any subsequent calls to locking functions from any - thread using {\tt ptr} have undefined effects. This also - applies to any thread currently calling {\tt upc\_lock}. +\np Subsequent or concurrent calls from any thread to functions defined in Section\ref{upc_lock}

+ using the lock referenced by {\tt ptr} have undefined behavior.% + \footnote{This also applies to any call to {\tt upc\_lock} on the the lock referenced

+ by {\tt ptr} which is blocked at the time of the call to {\tt upc\_lock\_free}.}

\paragraph{The {\tt upc\_lock} function}

```

Reported by `danbonachea` on 2012-08-13 04:22:41

[Former user]

``` With the introduction (see issue #12) of the collective lock deallocation call upc_all_lock_free(), the use of "or concurrent" in the proposed clarification may instead produce confusion. ```

Reported by `[e-mail]` on 2012-08-13 05:41:40

[Former user]

``` "With the introduction (see issue #12) of the collective lock deallocation call upc_all_lock_free(), the use of "or concurrent" in the proposed clarification may instead produce confusion."

I disagree - the proposed language is only for the existing, non-collective upc_lock_free() function. Any other concurrent calls using the same lock (even some thread entering upc_all_lock_free() while this thread calls upc_lock_free()) is an error.

The spec for upc_all_lock_free() will add a new section with its own language (yet to be written), but I don't see how that conflicts with this.

```

Reported by `danbonachea` on 2012-08-13 06:41:16

[Former user]

``` Dan's comment #7 is entirely correct. Please disregard my comment #6, which is nonsense.

```

Reported by `[e-mail]` on 2012-08-13 06:49:49

[Former user]

``` Mass-edit to mark all issues with an officially-announced resolution diff as "PendingApproval" ```

Reported by `danbonachea` on 2012-08-17 15:25:01 - Status changed: `PendingApproval`

[Former user]

``` Set Consensus:High on Pending Approval issues. ```

Reported by `gary.funck` on 2012-08-19 23:31:54 - Labels added: Consensus-High

[Former user]

``` Comment #5 wording applied in SVN r106 ```

Reported by `danbonachea` on 2012-09-13 23:52:26 - Status changed: `Fixed`

[Former user]

``` For the record: Public comment period for this change was 8/14/2012 - 9/14/2012 No substantial objections were raised or recorded during that period. At the 9/7/2012 telecon, it was announced this change was imminent, feedback was explicitly solicited, and none was received. The change was integrated into a working draft distributed on 9/13/2012 for consideration and draft ratification at the 9/14/2012 telecon. ```

Reported by `danbonachea` on 2012-09-14 07:26:14

[Former user]

``` Changes distributed as Draft 1.1 were ratified during the 9/14/2012 teleconference, for inclusion in the next public draft. ```

Reported by `danbonachea` on 2012-09-14 21:46:01 - Status changed: `Ratified`