Github user ahgittin commented on the pull request:
https://github.com/apache/brooklyn-server/pull/53#issuecomment-194936727
good start, but i think the acquisition logic is too brittle.
if a machine fails or is cancelled or it somehow doesn't go through the
`customize(..., Location)` method the lock is kept forever. i'd suggest:
* in the `synchronized` block, before acquiring the semaphore, check
whether anyone has owned it for more than 30m, if so, forcibly eject them (we
have a class `SemaphoreWithOwners` which could be extended to include timestamp
of when owning threads were issued it to achieve this)
* if it's easy to check for creation failure we should `release` if we were
the owner (or simply if there are no permits available)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
