Added ServerID function #107
Conversation
| } | ||
|
|
||
| // Cleanup | ||
| l.locked = false |
There was a problem hiding this comment.
I think here is a data race: You first remove the key in the agency, then set locked to false (irrelevant here), and only then stop the renewal. I suggest to first cancel the renewal, make sure it is cancelled (unless l.cancelRenewal() does this automatically), then remove the key and only then set locked to false.
There was a problem hiding this comment.
OK, now I understand, the renewal has a precondition as well. OK, this is good. Nevertheless, I think the change of order makes the code easier understandable. Furthermore, If the RemoveKeyIfEqualTo fails, then the renewal go routine might keep running. If we have cancelled first, it is gone in any case.
There was a problem hiding this comment.
Note that this function (and the renewal) first claim an in-memory mutex.
So there can be no race.
Second. If RemoveKeyIfEqualTo fails, the lock is likely still held, so I expect the renewal to continue.
This PR extends #106