camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Claus Ibsen <claus.ib...@gmail.com>
Subject Re: svn commit: r892199 - /camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java
Date Fri, 18 Dec 2009 14:40:22 GMT
On Fri, Dec 18, 2009 at 3:31 PM, William Tam <email.wtam@gmail.com> wrote:
>
>
> Claus Ibsen wrote:
>>
>> Hi
>>
>> Actually the AtomicBoolean cannot safely be used for initializing code
>> blocks?
>>
>
> I thought object construction and member initialization were atomic and
> threadsafe but I am not an expert on that.
>>

There are only a dozens expert on this matter, and all here are not experts.
However Brian Goetz is such an expert and his book - Java concurrency
in practice is a really good book.

AtomicBoolean is just a volatile boolean that I guess is a bit
optimized and it got that compareAndSet method that is guaranteed to
happen atomic among threads.

Anyway we had such a case with the camel-cxf component some months ago
that had this bug, and causes a NPE under heavy load on startup.
We fixed it by moving the initalization to the service doStart method.




>> You can still have concurrent threads invoking the boolean and one
>> thread will see it as false and the other as true.
>> What it helps is that there are only one ever that sees it as false
>> and therefore only one thread that will initialize it.
>>
>> But in the mean time another thread could have seen it as true, but
>> its still not initialized because the first thread is currently doing
>> that.
>> e.g. there are no locks.
>>
>> You have to use the synchronized or a Lock object.
>>
>> See for example camel-jaxb and how it use the synchronized.
>>
>> Its not bad to use as synchronized is heavily optimized in JDK6.
>>
>>
>> A better idea would be to implements Service (extends ServiceSupport)
>> and do initialization in the start method.
>> I am pretty sure Camel will invoke these start/stop on data formats as
>> well. Could you try that? If not we should get that done.
>> Then all kind of initialization can be done using the Service interface.
>>
>>
>>
>>
>> On Fri, Dec 18, 2009 at 10:54 AM,  <ningjiang@apache.org> wrote:
>>
>>>
>>> Author: ningjiang
>>> Date: Fri Dec 18 09:54:30 2009
>>> New Revision: 892199
>>>
>>> URL: http://svn.apache.org/viewvc?rev=892199&view=rev
>>> Log:
>>> CAMEL-2148 using the ClassResolver from the CamelContext to load class
>>>
>>> Modified:
>>>
>>> camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java
>>>
>>> Modified:
>>> camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java
>>> URL:
>>> http://svn.apache.org/viewvc/camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java?rev=892199&r1=892198&r2=892199&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java
>>> (original)
>>> +++
>>> camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java
>>> Fri Dec 18 09:54:30 2009
>>> @@ -20,10 +20,12 @@
>>>  import java.io.InputStream;
>>>  import java.io.OutputStream;
>>>  import java.lang.reflect.Method;
>>> +import java.util.concurrent.atomic.AtomicBoolean;
>>>
>>>  import com.google.protobuf.Message;
>>>  import com.google.protobuf.Message.Builder;
>>>
>>> +import org.apache.camel.CamelContext;
>>>  import org.apache.camel.CamelException;
>>>  import org.apache.camel.Exchange;
>>>  import org.apache.camel.InvalidPayloadException;
>>> @@ -34,6 +36,8 @@
>>>  public class ProtobufDataFormat implements DataFormat {
>>>
>>>    private Message defaultInstance;
>>> +    private String instanceClassName;
>>> +    private AtomicBoolean setDefaultInstanceHasBeenCalled = new
>>> AtomicBoolean(false);
>>>
>>>    /**
>>>     * @param defaultInstance
>>> @@ -51,11 +55,15 @@
>>>
>>>    public void setInstanceClass(String className) throws Exception {
>>>        ObjectHelper.notNull(className, "ProtobufDataFormat
>>> instaceClass");
>>> -        Class<?> instanceClass = ObjectHelper.loadClass(className);
>>> +        instanceClassName = className;
>>> +    }
>>> +
>>> +    protected Message loadDefaultInstance(String className, CamelContext
>>> context) throws CamelException, ClassNotFoundException {
>>> +        Class<?> instanceClass =
>>> context.getClassResolver().resolveMandatoryClass(className);
>>>        if (Message.class.isAssignableFrom(instanceClass)) {
>>>            try {
>>>                Method method =
>>> instanceClass.getMethod("getDefaultInstance", new Class[0]);
>>> -                defaultInstance = (Message) method.invoke(null, new
>>> Object[0]);
>>> +                return (Message) method.invoke(null, new Object[0]);
>>>            } catch (Exception ex) {
>>>                throw new CamelException("Can't set the defaultInstance
of
>>> ProtobufferDataFormat with "
>>>                                         + className + ",
caused by " +
>>> ex);
>>> @@ -64,7 +72,6 @@
>>>            throw new CamelException("Can't set the defaultInstance of
>>> ProtobufferDataFormat with "
>>>                  + className + ", as the class is not a subClass of
>>> com.google.protobuf.Message");
>>>        }
>>> -
>>>    }
>>>
>>>    /*
>>> @@ -82,8 +89,15 @@
>>>     * java.io.InputStream)
>>>     */
>>>    public Object unmarshal(Exchange exchange, InputStream inputStream)
>>> throws Exception {
>>> +
>>>        if (this.defaultInstance == null) {
>>> -            throw new CamelException("There is not defaultInstance for
>>> protobuf unmarshaling");
>>> +            if (instanceClassName == null) {
>>> +                throw new CamelException("There is not defaultInstance
>>> for protobuf unmarshaling");
>>> +            } else {
>>> +                if (!setDefaultInstanceHasBeenCalled.getAndSet(true))
{
>>> +                    defaultInstance =
>>> loadDefaultInstance(instanceClassName, exchange.getContext());
>>> +                }
>>> +            }
>>>        }
>>>        Builder builder =
>>> this.defaultInstance.newBuilderForType().mergeFrom(inputStream);
>>>        if (!builder.isInitialized()) {
>>>
>>>
>>>
>>>
>>
>>
>>
>>
>



-- 
Claus Ibsen
Apache Camel Committer

Author of Camel in Action: http://www.manning.com/ibsen/
Open Source Integration: http://fusesource.com
Blog: http://davsclaus.blogspot.com/
Twitter: http://twitter.com/davsclaus

Mime
View raw message