Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1765#discussion_r88391404
--- Diff:
plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java
---
@@ -100,36 +100,25 @@ protected SR createFileSR(final Connection conn,
final String path) {
PBD pbd = null;
try {
- final String srname = hypervisorResource.getHost().getUuid() +
path.trim();
-
- final Set<SR> srs = SR.getByNameLabel(conn, srname);
-
- if (srs != null && !srs.isEmpty()) {
- return srs.iterator().next();
+ final String srname = path.trim();
+ synchronized (srname.intern()) {
--- End diff --
Should we consider using a `ReentrantLock` instead of `synchronized`?
`ReentrantLock.tryLock(long, TimeUnit)` allows the specification of a timeout
to acquire the lock. Since a timeout cannot be specified to `synchronized`,
multiple threads may deadlock attempting to acquire the lock in the case of a
stall. The downside of this approach is that we would need to track locks in a
`ConcurrentMap` -- ideally with WeakReferences to allow them to be garbage
collected when no threads are operating on them.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
