commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Phil Steitz (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LANG-762) Handle or document ReflectionToStringBuilder and ToStringBuilder for collections that are not thread safe
Date Thu, 10 Nov 2011 03:29:51 GMT

    [ 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

        

Mime
View raw message