From 4528649b49315f5527f52ff2b89f68cad84f99de Mon Sep 17 00:00:00 2001 From: extio-de Date: Sun, 4 Oct 2026 14:33:38 +0200 Subject: [PATCH] fix(RendererWorkingSetImpl): break lock-order deadlock between commit and get --- .../renderer/work/RendererWorkingSetImpl.java | 81 ++++++++++--------- 1 file changed, 45 insertions(+), 36 deletions(-) diff --git a/src/main/java/de/extio/game_engine/renderer/work/RendererWorkingSetImpl.java b/src/main/java/de/extio/game_engine/renderer/work/RendererWorkingSetImpl.java index 97d1bc7..d542ac1 100644 --- a/src/main/java/de/extio/game_engine/renderer/work/RendererWorkingSetImpl.java +++ b/src/main/java/de/extio/game_engine/renderer/work/RendererWorkingSetImpl.java @@ -18,6 +18,8 @@ public class RendererWorkingSetImpl implements RendererWorkingSet { private final ConcurrentMap workingSet = new ConcurrentHashMap<>(); + private final ConcurrentMap commitLocks = new ConcurrentHashMap<>(); + private final BlockingQueue> mapsPool = new ArrayBlockingQueue<>(100); private final RenderingBoPool rendererBoPool; @@ -82,7 +84,7 @@ public class RendererWorkingSetImpl implements RendererWorkingSet { public RenderingBo get(final String producerId, final String id) { final var next = this.getWorkingSetByProducer(producerId).next(); synchronized (next) { - final var bo = this.getWorkingSetByProducer(producerId).next().get(id); + final var bo = next.get(id); if (bo == null) { return null; } @@ -99,7 +101,7 @@ public class RendererWorkingSetImpl implements RendererWorkingSet { public T get(final String producerId, final String id, final Class type) { final var next = this.getWorkingSetByProducer(producerId).next(); synchronized (next) { - final RenderingBo bo = this.getWorkingSetByProducer(producerId).next().get(id); + final RenderingBo bo = next.get(id); if (bo == null) { return null; } @@ -123,38 +125,38 @@ public class RendererWorkingSetImpl implements RendererWorkingSet { @Override public Map commit(final String producerId, final boolean clone) { - final AtomicReference> previousLiveRef = new AtomicReference<>(); - final RendererWork rendererWork = this.workingSet.compute(producerId, (k, v) -> { - if (v == null) { - return new RendererWork(this.obtainMapFromPool(), this.obtainMapFromPool()); - } - previousLiveRef.set(v.live()); - if (clone) { - final Map newNext = this.obtainMapFromPool(); - synchronized (v.next()) { - v.next().entrySet().forEach(e -> { - final RenderingBo copy = this.rendererBoPool.copy(e.getValue()); - newNext.put(e.getKey(), copy); - }); + synchronized (this.commitLockFor(producerId)) { + final AtomicReference previousWorkRef = new AtomicReference<>(); + final RendererWork rendererWork = this.workingSet.compute(producerId, (k, v) -> { + if (v == null) { + return new RendererWork(this.obtainMapFromPool(), this.obtainMapFromPool()); } - return new RendererWork(v.next(), newNext); - } - else { + previousWorkRef.set(v); return new RendererWork(v.next(), this.obtainMapFromPool()); - } - }); - - final var previousLiveSet = previousLiveRef.get(); - if (previousLiveSet != null) { - if (clone) { - synchronized (previousLiveSet) { - previousLiveSet.values().forEach(this.rendererBoPool::returnToPool); + }); + + final var previousWork = previousWorkRef.get(); + if (previousWork != null) { + if (clone) { + final var oldNext = previousWork.next(); + synchronized (oldNext) { + oldNext.entrySet().forEach(e -> { + final RenderingBo copy = this.rendererBoPool.copy(e.getValue()); + rendererWork.next().put(e.getKey(), copy); + }); + } } + final var previousLiveSet = previousWork.live(); + if (clone) { + synchronized (previousLiveSet) { + previousLiveSet.values().forEach(this.rendererBoPool::returnToPool); + } + } + this.returnMapToPool(previousLiveSet); } - this.returnMapToPool(previousLiveSet); + + return rendererWork.next(); } - - return rendererWork.next(); } @Override @@ -168,15 +170,18 @@ public class RendererWorkingSetImpl implements RendererWorkingSet { @Override public void clear(final String producerId) { - final RendererWork rendererWork = this.workingSet.remove(producerId); - if (rendererWork != null) { - synchronized (rendererWork.live()) { - rendererWork.live().values().forEach(this.rendererBoPool::returnToPool); - } - synchronized (rendererWork.next()) { - rendererWork.next().values().forEach(this.rendererBoPool::returnToPool); + synchronized (this.commitLockFor(producerId)) { + final RendererWork rendererWork = this.workingSet.remove(producerId); + if (rendererWork != null) { + synchronized (rendererWork.live()) { + rendererWork.live().values().forEach(this.rendererBoPool::returnToPool); + } + synchronized (rendererWork.next()) { + rendererWork.next().values().forEach(this.rendererBoPool::returnToPool); + } } } + this.commitLocks.remove(producerId); } @Override @@ -194,6 +199,10 @@ public class RendererWorkingSetImpl implements RendererWorkingSet { return this.workingSet.computeIfAbsent(producerId, k -> new RendererWork(this.obtainMapFromPool(), this.obtainMapFromPool())); } + private Object commitLockFor(final String producerId) { + return this.commitLocks.computeIfAbsent(producerId, k -> new Object()); + } + private Map obtainMapFromPool() { var map = this.mapsPool.poll(); if (map == null) {