[SLING-12344] Lock contention in ScriptDependencyResolver

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: Scripting HTL Engine 1.4.22-1.4.0
• Fix Version/s: Scripting HTL Testing Content 1.0.32-1.4.0, Scripting HTL Engine 1.4.24-1.4.0, Scripting HTL Testing 1.0.34-1.4.0
• Component/s: HTL
• Assignee: Radu Cotescu
• Reporter: Joerg Hoh
• Created: 03/Jun/24 09:31
• Updated: 10/Jul/24 14:49
• Resolved: 05/Jul/24 12:20

Description

I see threaddumps which show lock contention in the ScriptDependencyResolver like this:

        at jdk.internal.misc.Unsafe.park(java.base@11.0.20/Native Method)
        - parking to wait for  <0x0000000496e16af0> (a java.util.concurrent.locks.ReentrantReadWriteLock$NonfairSync)
        at java.util.concurrent.locks.LockSupport.park(java.base@11.0.20/LockSupport.java:194)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(java.base@11.0.20/AbstractQueuedSynchronizer.java:885)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued(java.base@11.0.20/AbstractQueuedSynchronizer.java:917)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(java.base@11.0.20/AbstractQueuedSynchronizer.java:1240)
        at java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock.lock(java.base@11.0.20/ReentrantReadWriteLock.java:959)
        at org.apache.sling.scripting.sightly.impl.utils.ScriptDependencyResolver.resolveScript(ScriptDependencyResolver.java:100)
        at org.apache.sling.scripting.sightly.impl.engine.extension.use.RenderUnitProvider.provide(RenderUnitProvider.java:95)
        at org.apache.sling.scripting.sightly.impl.engine.extension.use.UseRuntimeExtension.call(UseRuntimeExtension.java:71)
        at org.apache.sling.scripting.sightly.impl.engine.runtime.RenderContextImpl.call(RenderContextImpl.java:72)
        at apps.components.x.y.y__002e__html.render(y__002e__html.java:39)

but also:

        at jdk.internal.misc.Unsafe.park(java.base@11.0.20/Native Method)
        - parking to wait for  <0x0000000496e16af0> (a java.util.concurrent.locks.ReentrantReadWriteLock$NonfairSync)
        at java.util.concurrent.locks.LockSupport.park(java.base@11.0.20/LockSupport.java:194)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(java.base@11.0.20/AbstractQueuedSynchronizer.java:885)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireShared(java.base@11.0.20/AbstractQueuedSynchronizer.java:1009)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireShared(java.base@11.0.20/AbstractQueuedSynchronizer.java:1324)
        at java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock.lock(java.base@11.0.20/ReentrantReadWriteLock.java:738)
        at org.apache.sling.scripting.sightly.impl.utils.ScriptDependencyResolver.resolveScript(ScriptDependencyResolver.java:93)
        at org.apache.sling.scripting.sightly.impl.engine.extension.use.RenderUnitProvider.provide(RenderUnitProvider.java:95)
        at org.apache.sling.scripting.sightly.impl.engine.extension.use.UseRuntimeExtension.call(UseRuntimeExtension.java:71)
        at org.apache.sling.scripting.sightly.impl.engine.runtime.RenderContextImpl.call(RenderContextImpl.java:72)
        at apps.components.a.b.b__002e__html$3.render(b__002e__html.java:218)

I see that the ScriptDependencyResolver holds cache, which just saves positive results where the result has been found; if the result is null, it's not cached, and it will be attempted over and over again.

Of course this situation mostly happens if a lot of these requests with invalid scriptIdentifiers are done, which points to problems in the content. But it would be great if the code behaves a bit better here, because I have a situation here where requests holding the write lock block all other requests which would have a cache hit.

(Thinking further, I don't really understand why a ReadWrite lock is used here at all. As far as I can see a ConcurrentHashMap as a cache should be sufficient.)

Issue Links

• relates to: SLING-11608 Cache script dependency resolution
• links to: GitHub Pull Request #26