commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Phil Steitz (Updated) (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (LANG-762) Handle or document ReflectionToStringBuilder and ToStringBuilder for collections that are not thread safe
Date Sat, 22 Oct 2011 07:12:32 GMT

     [ https://issues.apache.org/jira/browse/LANG-762?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Phil Steitz updated LANG-762:
-----------------------------

    Attachment: ReflectionToStringBuilderConcurrencyTest.java

Attaching a test that illustrates the problem, which I don't think is solvable by the [lang]
code (so probably this class should not be committed, as it will never pass).

The problem is that when you circumvent a class' internal synchronization to protect fields,
all bets are off in terms of data integrity or CoMod exceptions. This should be clearly documented
- i.e., users should be warned that this class should not be used in concurrent applications,
or at least fields protected by synchronization should be excluded.

What is going on in the test case is that there are two different kinds of threads spawned
concurrently - one kind uses the reflection back door opened by the builder to "inspect" an
instance and the other kind mutates the instance using its synchronized methods.  Note that
synchronizing the builder using the private field's monitor will not solve the problem (i.e.,
the first patch does not work) because what really needs to happen is that the access by the
builder needs to be synchronized using the instance's monitor.  You could try to fix that
by synchronizing on the instance, which would solve this example; but there is no guarantee
that a class may not use its own internal locks, so there is no generic solution to this problem.

My recommendation is to just document the danger associated with using reflection to access
private fields in the class javadoc.

                
> 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