Inadequate synchronization in lucee.runtime.reflection.storage.SoftMethodStorage

Description

(issue observed in 4.5.3.020, but review of available code on GitHub indicates that this problem is common to Railo, Lucee 4, and Lucee 5)

SoftMethodStorage.storeMethod uses the 'synchronized' keyword, but it's the only method in the class which does - so there's no protection against reading from the collection while it's being modified.

This situation is unlikely, but one of our customers hit it yesterday, resulting in infinite looping and tons of log entries with this stack trace:
0: ........org.apache.commons.collections.map.AbstractHashedMap.getEntry[AbstractHashedMap.java:440]
1: ........org.apache.commons.collections.map.AbstractReferenceMap.getEntry[AbstractReferenceMap.java:405]
2: ........org.apache.commons.collections.map.AbstractReferenceMap.get[AbstractReferenceMap.java:230]
3: ........lucee.runtime.reflection.storage.SoftMethodStorage.getMethods[SoftMethodStorage.java:47]
4: ........lucee.runtime.reflection.Reflector.getMethodInstanceEL[Reflector.java:491]
5: ........lucee.runtime.reflection.Reflector.callMethod[Reflector.java:853]
6: ........lucee.runtime.util.VariableUtilImpl.callFunctionWithoutNamedValues[VariableUtilImpl.java:757]
7: ........lucee.runtime.PageContextImpl.getFunction[PageContextImpl.java:1593]
8: CFM....{our CF code attempting to call a method in one of our Java classes)

============

The easy fix (add 'synchronized' to all methods which access the 'map' member) is less than ideal, since it disallows simultaneous reads - but it would eliminate the infinite loop risk

Environment

CFML Engine Version: Lucee 4.5.3.020 (Apache Tomcat/8.0.38)
JVM version: 1.8.0_112-b15 - [64 bits, Linux]
JVM memory (MB): max:2543.38 total:1519.38 free:1404.64

Activity

Show:

Tim Parker 23 June 2020 at 16:18

A quick review of the file Zac referenced indicates that this should be resolved in 5.3.4.7 or later (July 22, 2019 - commit comment ‘make sure ReferenceMap used by multiple threads are thread safe.’). I guess this is as good a reason as any to advocate for moving sites to a newer Lucee release.

Zac Spitzer 23 June 2020 at 09:45

There's been a lot of work on synchronisation since these older versions of Lucee, this has possibly already been addressed

https://github.com/lucee/Lucee/commits/5.3/core/src/main/java/lucee/runtime/reflection/storage/SoftMethodStorage.java

Harry Klein 23 June 2020 at 09:04
Edited

We had the same issue today which caused all requests to hang, and brought the server down!

Code:

#listToArray(lContentLanguages).toString()#


Stack:

org.apache.commons.collections4.map.AbstractHashedMap.getEntry(AbstractHashedMap.java:461) org.apache.commons.collections4.map.AbstractReferenceMap.getEntry(AbstractReferenceMap.java:427) org.apache.commons.collections4.map.AbstractReferenceMap.get(AbstractReferenceMap.java:244) lucee.runtime.reflection.storage.SoftMethodStorage.getMethods(SoftMethodStorage.java:48) lucee.runtime.reflection.Reflector.getMethodInstanceEL(Reflector.java:490) lucee.runtime.type.util.MemberUtil.callMethod(MemberUtil.java:156) lucee.runtime.type.util.MemberUtil.call(MemberUtil.java:126) lucee.runtime.type.util.ArraySupport.call(ArraySupport.java:341) lucee.runtime.util.VariableUtilImpl.callFunctionWithoutNamedValues(VariableUtilImpl.java:756) lucee.runtime.PageContextImpl.getFunction(PageContextImpl.java:1716) ui.views.basics.form_header_cfm$cf.call(/contenscms/ui/views/basics/form_header.cfm:141)

Lucee: 5.2.8.50

Tim Parker 15 June 2017 at 17:31

update: I missed a critical detail here - the quick fix is still the same, but the critical issue is that the 'map' member is not protected by synchronization - there is no protection against simultaneous 'put' calls on the 'map' member.

Unresolved

Details

Assignee

Reporter

Priority

Fix versions

New Issue warning screen

Before you create a new Issue, please post to the mailing list first https://dev.lucee.org

Once the issue has been verified, one of the Lucee team will ask you to file an issue

Affects versions

Created 15 June 2017 at 17:06
Updated 6 March 2021 at 00:59

Flag notifications