Skip to content

Commit

Permalink
Merge pull request #114 from openbase/feature/113-registry-startup-de…
Browse files Browse the repository at this point in the history 
…adlock

Reduce deadlock potential in FileSynchronizedRegistryImpl and Audio Engine improvements
  • Loading branch information
@DivineThreepwood
DivineThreepwood committed Feb 20, 2024
2 parents 6265f90 + 9a023a6 commit 69be6a7
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 46 deletions.
8 changes: 7 additions & 1 deletion module/audio/src/main/java/org/openbase/jul/audio/AudioPlayer.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ import kotlin.concurrent.withLock
* @author [Divine Threepwood](mailto:divine@openbase.org)
*/
class AudioPlayer @JvmOverloads constructor(soundChannels: Int = 10) {
private val executorService: ExecutorService = Executors.newFixedThreadPool(soundChannels)
private val executorService: ExecutorService = Executors.newFixedThreadPool(soundChannels) { runnable: Runnable ->
Executors.defaultThreadFactory().newThread(runnable)
.apply {
isDaemon = true
priority = Thread.MAX_PRIORITY.dec()
}
}

@JvmOverloads
fun playAudio(source: AudioSource, wait: Boolean = false): Boolean {
Expand Down
81 changes: 36 additions & 45 deletions ...storage/src/main/java/org/openbase/jul/storage/registry/FileSynchronizedRegistryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,16 +155,16 @@ public void activateVersionControl(final String entryType, final Package convert
@Override
public ENTRY register(final ENTRY entry) throws CouldNotPerformException {
try {
ENTRY result = super.register(entry);
FileSynchronizer<ENTRY> fileSynchronizer = new FileSynchronizer<>(result, new File(databaseDirectory, fileProvider.getFileName(entry)), FileSynchronizer.InitMode.CREATE, fileProcessor);
fileSynchronizerMapLock.writeLock().lockInterruptibly();
try {
ENTRY result = super.register(entry);
FileSynchronizer<ENTRY> fileSynchronizer = new FileSynchronizer<>(result, new File(databaseDirectory, fileProvider.getFileName(entry)), FileSynchronizer.InitMode.CREATE, fileProcessor);
fileSynchronizerMap.put(result.getId(), fileSynchronizer);
filePluginPool.afterRegister(result, fileSynchronizer);
return result;
} finally {
fileSynchronizerMapLock.writeLock().unlock();
}
filePluginPool.afterRegister(result, fileSynchronizer);
return result;
} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
throw new RuntimeException(ex);
Expand All @@ -174,26 +174,26 @@ public ENTRY register(final ENTRY entry) throws CouldNotPerformException {
@Override
public ENTRY update(final ENTRY entry) throws CouldNotPerformException {
try {
ENTRY result = super.update(entry);
FileSynchronizer<ENTRY> fileSynchronizer;
fileSynchronizerMapLock.readLock().lockInterruptibly();
try {
ENTRY result = super.update(entry);

// ignore update during registration process.
if (!fileSynchronizerMap.containsKey(result.getId())) {
logger.debug("Ignore update during registration process of entry " + result);
return entry;
}

FileSynchronizer<ENTRY> fileSynchronizer = fileSynchronizerMap.get(result.getId());

filePluginPool.beforeUpdate(result, fileSynchronizer);
fileSynchronizer.save(result);
filePluginPool.afterUpdate(result, fileSynchronizer);

return result;
fileSynchronizer = fileSynchronizerMap.get(result.getId());
} finally {
fileSynchronizerMapLock.readLock().unlock();
}
filePluginPool.beforeUpdate(result, fileSynchronizer);
fileSynchronizer.save(result);
filePluginPool.afterUpdate(result, fileSynchronizer);

return result;

} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
throw new RuntimeException(ex);
Expand All @@ -203,21 +203,21 @@ public ENTRY update(final ENTRY entry) throws CouldNotPerformException {
@Override
public ENTRY remove(final ENTRY entry) throws CouldNotPerformException {
try {
ENTRY removedValue = super.remove(entry);
FileSynchronizer<ENTRY> fileSynchronizer;

fileSynchronizerMapLock.writeLock().lockInterruptibly();
try {
ENTRY removedValue = super.remove(entry);

FileSynchronizer<ENTRY> fileSynchronizer = fileSynchronizerMap.get(entry.getId());

filePluginPool.beforeRemove(entry, fileSynchronizer);
fileSynchronizer.delete();
fileSynchronizerMap.remove(entry.getId());
filePluginPool.afterRemove(entry, fileSynchronizer);

return removedValue;
fileSynchronizer = fileSynchronizerMap.get(entry.getId());
} finally {
fileSynchronizerMapLock.writeLock().unlock();
}

filePluginPool.beforeRemove(entry, fileSynchronizer);
fileSynchronizer.delete();
fileSynchronizerMap.remove(entry.getId());
filePluginPool.afterRemove(entry, fileSynchronizer);
return removedValue;
} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
throw new RuntimeException(ex);
Expand Down Expand Up @@ -309,15 +309,17 @@ public void loadRegistry() throws CouldNotPerformException {

// init file synchronizer
try {
FileSynchronizer<ENTRY> fileSynchronizer = new FileSynchronizer<>(file, fileProcessor);
ENTRY entry = fileSynchronizer.getData();

fileSynchronizerMapLock.writeLock().lockInterruptibly();
try {
FileSynchronizer<ENTRY> fileSynchronizer = new FileSynchronizer<>(file, fileProcessor);
ENTRY entry = fileSynchronizer.getData();
fileSynchronizerMap.put(entry.getId(), fileSynchronizer);
super.load(entry);
} finally {
fileSynchronizerMapLock.writeLock().unlock();
}

super.load(entry);
} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
throw new RuntimeException(ex);
Expand All @@ -341,7 +343,7 @@ public void loadRegistry() throws CouldNotPerformException {
}

@Override
public synchronized void saveRegistry() throws MultiException {
public void saveRegistry() throws MultiException {

if (JPService.testMode()) {
return;
Expand All @@ -357,9 +359,9 @@ public synchronized void saveRegistry() throws MultiException {
logger.debug("Save " + this + " into " + databaseDirectory + "...");
ExceptionStack exceptionStack = null;

// save all changes.
try {
fileSynchronizerMapLock.readLock().lockInterruptibly();
// save all changes.
fileSynchronizerMapLock.writeLock().lockInterruptibly();
try {
for (FileSynchronizer<ENTRY> fileSynchronizer : new ArrayList<>(fileSynchronizerMap.values())) {
try {
Expand All @@ -368,24 +370,13 @@ public synchronized void saveRegistry() throws MultiException {
exceptionStack = MultiException.push(this, ex, exceptionStack);
}
}
} finally {
fileSynchronizerMapLock.readLock().unlock();
}
} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
throw new RuntimeException(ex);
}

// verify and apply file name changes
String generatedFileName;
FileSynchronizer<ENTRY> fileSynchronizer;
FileSynchronizer<ENTRY> newFileSynchronizer;
File newFile;

// verify and apply file name changes
String generatedFileName;
FileSynchronizer<ENTRY> fileSynchronizer;
FileSynchronizer<ENTRY> newFileSynchronizer;
File newFile;

try {
fileSynchronizerMapLock.writeLock().lockInterruptibly();
try {
for (Entry<KEY, FileSynchronizer<ENTRY>> entry : new ArrayList<>(fileSynchronizerMap.entrySet())) {
fileSynchronizer = entry.getValue();
try {
Expand Down

0 comments on commit 69be6a7

Please sign in to comment.