andreacavalli/scylla-jmx
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge 'Fix locking in APIBuilder.remove()' from Pekka Enberg

Browse Source 
This pull request reverts the commit c2fc96b ("APIBuilder: Remove
RW-lock in JMX server repository wrapper") and fixes a missing unlock
from APIBuilder.remove().

Closes #163

* github.com:scylladb/scylla-jmx:
  APIBuilder: Unlock RW-lock in remove()
  Revert "APIBuilder: Remove RW-lock in JMX server repository wrapper"
This commit is contained in:
Pekka Enberg 2021-03-03 18:29:57 +02:00
commit bac7d0b31e
1 changed files with 25 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/main/java/com/scylladb/jmx/utils/APIBuilder.java
View File

@ -66,7 +66,7 @@ public class APIBuilder extends MBeanServerBuilder {
private final Repository wrapped; private final Repository wrapped;
private final Object lock = new Object(); private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
private final Map<TableMetricParams, DynamicMBean> tableMBeans = new HashMap<>(); private final Map<TableMetricParams, DynamicMBean> tableMBeans = new HashMap<>();
@ -93,8 +93,11 @@ public class APIBuilder extends MBeanServerBuilder {
if (!isTableMetricName(name)) { if (!isTableMetricName(name)) {
return wrapped.contains(name); return wrapped.contains(name);
} else { } else {
synchronized(lock) { lock.readLock().lock();
try {
return tableMBeans.containsKey(new TableMetricParams(name)); return tableMBeans.containsKey(new TableMetricParams(name));
} finally {
lock.readLock().unlock();
} }
} }
} }
@ -113,20 +116,23 @@ public class APIBuilder extends MBeanServerBuilder {
@Override @Override
public Integer getCount() { public Integer getCount() {
synchronized(lock) { lock.readLock().lock();
try {
return wrapped.getCount() + tableMBeans.size(); return wrapped.getCount() + tableMBeans.size();
} finally {
lock.readLock().unlock();
} }
} }
@Override @Override
public void addMBean(final DynamicMBean bean, final ObjectName name, final RegistrationContext ctx) public void addMBean(final DynamicMBean bean, final ObjectName name, final RegistrationContext ctx)
throws InstanceAlreadyExistsException { throws InstanceAlreadyExistsException {
System.out.println(name);
if (!isTableMetricName(name)) { if (!isTableMetricName(name)) {
wrapped.addMBean(bean, name, ctx); wrapped.addMBean(bean, name, ctx);
} else { } else {
final TableMetricParams key = new TableMetricParams(name); final TableMetricParams key = new TableMetricParams(name);
synchronized(lock) { lock.writeLock().lock();
try {
if (tableMBeans.containsKey(key)) { if (tableMBeans.containsKey(key)) {
throw new InstanceAlreadyExistsException(name.toString()); throw new InstanceAlreadyExistsException(name.toString());
} }
@ -139,6 +145,8 @@ public class APIBuilder extends MBeanServerBuilder {
} catch (RuntimeException x) { } catch (RuntimeException x) {
throw new RuntimeOperationsException(x); throw new RuntimeOperationsException(x);
} }
} finally {
lock.writeLock().unlock();
} }
} }
} }
@ -149,7 +157,8 @@ public class APIBuilder extends MBeanServerBuilder {
wrapped.remove(name, ctx); wrapped.remove(name, ctx);
} else { } else {
final TableMetricParams key = new TableMetricParams(name); final TableMetricParams key = new TableMetricParams(name);
synchronized(lock) { lock.writeLock().lock();
try {
if (tableMBeans.remove(key) == null) { if (tableMBeans.remove(key) == null) {
throw new InstanceNotFoundException(name.toString()); throw new InstanceNotFoundException(name.toString());
} }
@ -162,6 +171,8 @@ public class APIBuilder extends MBeanServerBuilder {
} catch (Exception x) { } catch (Exception x) {
logger.log(SEVERE, "Unexpected error.", x); logger.log(SEVERE, "Unexpected error.", x);
} }
} finally {
lock.writeLock().unlock();
} }
} }
} }
@ -171,8 +182,11 @@ public class APIBuilder extends MBeanServerBuilder {
if (!isTableMetricName(name)) { if (!isTableMetricName(name)) {
return wrapped.retrieve(name); return wrapped.retrieve(name);
} else { } else {
synchronized(lock) { lock.readLock().lock();
try {
return tableMBeans.get(new TableMetricParams(name)); return tableMBeans.get(new TableMetricParams(name));
} finally {
lock.readLock().unlock();
} }
} }
} }
@ -215,7 +229,8 @@ public class APIBuilder extends MBeanServerBuilder {
name = pattern; name = pattern;
} }
synchronized(lock) { lock.readLock().lock();
try {
// If pattern is not a pattern, retrieve this mbean ! // If pattern is not a pattern, retrieve this mbean !
if (!name.isPattern() && isTableMetricName(name)) { if (!name.isPattern() && isTableMetricName(name)) {
final DynamicMBean bean = tableMBeans.get(new TableMetricParams(name)); final DynamicMBean bean = tableMBeans.get(new TableMetricParams(name));
@ -271,6 +286,8 @@ public class APIBuilder extends MBeanServerBuilder {
addAllMatching(res, namePattern); addAllMatching(res, namePattern);
} }
} }
} finally {
lock.readLock().unlock();
} }
return res; return res;
} }