[
https://issues.apache.org/jira/browse/LANG-762?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13147468#comment-13147468
]
Phil Steitz commented on LANG-762:
----------------------------------
Looking at this again, I am pretty well convinced that [lang] is never going to
be able to "fix" this issue. Consider the case where a class uses its own
private Lock instances to protect data members. Unless you want to get into
the byte code analysis business, you are not going to be able to pick this up
or access the relevant locks. Moreoever, even if you could discern and acquire
the right locks, you might risk introducing deadlocks or liveness problems for
the code, because the class may have lock acquisition / release order
invariants that you don't know about.
The risks associated with using the ReflectionToStringBuilder to access fields
protected by synchronization should be documented. I would suggest just adding
something like the following to the class javadoc, after the sentence that
reads "This will fail under a security manager, unless the appropriate
permissions are set up correctly" add "Using reflection to access private
fields also circumvents any synchronization protection guarding access to
private fields. Fields that cannot safely be read at any time by toString
should be excluded from the generated method, or synchronization consistent
with the underlying class' lock management should be added around invocation of
the method. Special care should be taken to avoid including non-threadsafe
collection classes, as these classes may throw ConcurrentModificationException
if modified while the toString method is executing."
Above is probably too long, but something like it should be added.
> Handle or document ReflectionToStringBuilder and ToStringBuilder for
> collections that are not thread safe
> ---------------------------------------------------------------------------------------------------------
>
> Key: LANG-762
> URL: https://issues.apache.org/jira/browse/LANG-762
> Project: Commons Lang
> Issue Type: Bug
> Components: lang.builder.*
> Environment: Apache Maven 3.0.3 (r1075438; 2011-02-28 12:31:09-0500)
> Maven home: C:\Java\apache-maven-3.0.3\bin\..
> Java version: 1.6.0_24, vendor: Sun Microsystems Inc.
> Java home: C:\Program Files\Java\jdk1.6.0_24\jre
> Default locale: en_US, platform encoding: Cp1252
> OS name: "windows 7", version: "6.1", arch: "amd64", family: "windows"
> Reporter: Gary D. Gregory
> Attachments: ReflectionToStringBuilderConcurrencyTest.java,
> patch-LANG762.txt
>
>
> Moving discussion here from https://issues.apache.org/jira/browse/POOL-191
> ConcurrentModificationException in GenericObjectPool LinkedList.
> It is possible to get a {{ConcurrentModificationException}} in a
> {{LinkedList}} from a Commons Pool {{GenericObjectPool}}.
> This happens when I call {{ReflectionToStringBuilder.toString(this)}} from a
> subclass of {{GenericObjectPool}}. My guess is that it would happen with just
> {{ReflectionToStringBuilder.toString(gop)}}. IOW, subclassing does not have
> anything to do with it I would venture.
> For example, in this stack trace {{JmsSessionPool}} is a subclass of
> {{GenericObjectPool}}.
> {noformat}
> java.util.ConcurrentModificationException
> at java.util.LinkedList$ListItr.checkForComodification(LinkedList.java:761)
> at java.util.LinkedList$ListItr.next(LinkedList.java:696)
> at java.util.AbstractCollection.toString(AbstractCollection.java:421)
> at java.lang.String.valueOf(String.java:2826)
> at java.lang.StringBuffer.append(StringBuffer.java:219)
> at
> org.apache.commons.lang3.builder.ToStringStyle.appendDetail(ToStringStyle.java:598)
> at
> org.apache.commons.lang3.builder.ToStringStyle.appendInternal(ToStringStyle.java:473)
> at
> org.apache.commons.lang3.builder.ToStringStyle.append(ToStringStyle.java:436)
> at
> org.apache.commons.lang3.builder.ToStringBuilder.append(ToStringBuilder.java:848)
> at
> org.apache.commons.lang3.builder.ReflectionToStringBuilder.appendFieldsIn(ReflectionToStringBuilder.java:528)
> at
> org.apache.commons.lang3.builder.ReflectionToStringBuilder.toString(ReflectionToStringBuilder.java:692)
> at
> org.apache.commons.lang3.builder.ReflectionToStringBuilder.toString(ReflectionToStringBuilder.java:288)
> at
> org.apache.commons.lang3.builder.ReflectionToStringBuilder.toString(ReflectionToStringBuilder.java:119)
> at
> com.seagullsw.appinterface.comm.jms.JmsSessionPool.toString(JmsSessionPool.java:120)
> at java.lang.String.valueOf(String.java:2826)
> at java.lang.StringBuffer.append(StringBuffer.java:219)
> at
> org.apache.commons.lang3.builder.ToStringStyle.appendDetail(ToStringStyle.java:586)
> at
> org.apache.commons.lang3.builder.ToStringStyle.appendInternal(ToStringStyle.java:550)
> at
> org.apache.commons.lang3.builder.ToStringStyle.append(ToStringStyle.java:436)
> at
> org.apache.commons.lang3.builder.ToStringBuilder.append(ToStringBuilder.java:848)
> at
> org.apache.commons.lang3.builder.ReflectionToStringBuilder.appendFieldsIn(ReflectionToStringBuilder.java:528)
> at
> org.apache.commons.lang3.builder.ReflectionToStringBuilder.toString(ReflectionToStringBuilder.java:689)
> at
> org.apache.commons.lang3.builder.ReflectionToStringBuilder.toString(ReflectionToStringBuilder.java:288)
> at
> org.apache.commons.lang3.builder.ReflectionToStringBuilder.toString(ReflectionToStringBuilder.java:119)
> at
> com.seagullsw.appinterface.server.comm.BasicCommunicationManager.toString(BasicCommunicationManager.java:828)
> at
> com.seagullsw.appinterface.server.comm.BasicCommunicationManager.toString(BasicCommunicationManager.java:817)
> at java.lang.String.valueOf(String.java:2826)
> at java.lang.StringBuilder.append(StringBuilder.java:115)
> at
> com.seagullsw.appinterface.server.AisHelper.waitForCommuncationManagers(AisHelper.java:217)
> at com.seagullsw.appinterface.server.AisHelper.start(AisHelper.java:136)
> at
> com.seagullsw.appinterface.server.AisHelper.startFromResource(AisHelper.java:161)
> at
> com.seagullsw.appinterface.server.AbstractServerJunit4.startServer(AbstractServerJunit4.java:179)
> at
> com.seagullsw.appinterface.server.comm.jms.AbstractJmsRoundtripMaxConcurrencyTestCase.setUpOnce(AbstractJmsRoundtripMaxConcurrencyTestCase.java:141)
> at
> com.seagullsw.appinterface.server.comm.jms.ibmmq.JmsRoundtripMaxConcurrency032TestCase.setUpOnce(JmsRoundtripMaxConcurrency032TestCase.java:40)
> {noformat}
> We should either:
> - Document ReflectionToStringBuilder and ToStringBuilder such that call sites
> use synchronized if the object passed in contains collections that are not
> thread-safe. F
> or example:
> {code:java}
> @Override
> public synchronized String toString() {
> return ReflectionToStringBuilder.toString(this);
> }
> {code}
> - Or have our code in ReflectionToStringBuilder and ToStringBuilder lock
> collections while they are being toString'd.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
