You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by Claus Ibsen <cl...@gmail.com> on 2009/12/18 11:20:30 UTC

Re: svn commit: r892199 - /camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java

Hi

Actually the AtomicBoolean cannot safely be used for initializing code blocks?

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,  <ni...@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

Re: svn commit: r892199 - /camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java

Posted by Willem Jiang <wi...@gmail.com>.
Hi Claus,

Thanks for point issue out.
I added a lock on loading the class, it should be better now ;)

Willem
Claus Ibsen wrote:
> On Fri, Dec 18, 2009 at 11:20 AM, Claus Ibsen <cl...@gmail.com> wrote:
>> Hi
>>
>> Actually the AtomicBoolean cannot safely be used for initializing code blocks?
>>
>> 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.
>>
> 
> Ah yeah you need to do it at runtime sine you need access to CamelContext.
> I wonder if implementing CamelContextAware is sufficient?
> 
> 
>>
>>
>> On Fri, Dec 18, 2009 at 10:54 AM,  <ni...@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
>>
> 
> 
> 


Re: svn commit: r892199 - /camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java

Posted by Claus Ibsen <cl...@gmail.com>.
On Fri, Dec 18, 2009 at 11:20 AM, Claus Ibsen <cl...@gmail.com> wrote:
> Hi
>
> Actually the AtomicBoolean cannot safely be used for initializing code blocks?
>
> 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.
>

Ah yeah you need to do it at runtime sine you need access to CamelContext.
I wonder if implementing CamelContextAware is sufficient?


>
>
>
> On Fri, Dec 18, 2009 at 10:54 AM,  <ni...@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
>



-- 
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

Re: svn commit: r892199 - /camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java

Posted by Claus Ibsen <cl...@gmail.com>.
On Fri, Dec 18, 2009 at 3:31 PM, William Tam <em...@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,  <ni...@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

Re: svn commit: r892199 - /camel/trunk/components/camel-protobuf/src/main/java/org/apache/camel/dataformat/protobuf/ProtobufDataFormat.java

Posted by William Tam <em...@gmail.com>.

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.
> 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,  <ni...@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()) {
>>
>>
>>
>>     
>
>
>
>