You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by Hadrian Zbarcea <hz...@gmail.com> on 2011/09/02 16:50:37 UTC
Fwd: Re: svn commit: r1164515 - /camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
Moving to dev@.
-1. Please revert your change and let's fix it properly. In the future
please do not revert my changes without discussing that with me, even if
you believe it's the right thing to do. The PropertyEditoryTypeConverter
and being a Service and leaks are two separate issues.
Hadrian
On 09/02/2011 08:51 AM, davsclaus@apache.org wrote:
> Author: davsclaus
> Date: Fri Sep 2 12:51:57 2011
> New Revision: 1164515
>
> URL: http://svn.apache.org/viewvc?rev=1164515&view=rev
> Log:
> CAMEL-4392. Clear cache maps when stopping Camel to avoid leaks
>
> Modified:
> camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
>
> Modified: camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
> URL: http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java?rev=1164515&r1=1164514&r2=1164515&view=diff
> ==============================================================================
> --- camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java (original)
> +++ camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java Fri Sep 2 12:51:57 2011
> @@ -22,6 +22,7 @@ import java.util.HashMap;
> import java.util.Map;
>
> import org.apache.camel.Exchange;
> +import org.apache.camel.Service;
> import org.apache.camel.TypeConverter;
> import org.apache.camel.util.LRUSoftCache;
> import org.apache.camel.util.ObjectHelper;
> @@ -34,7 +35,7 @@ import org.slf4j.LoggerFactory;
> *
> * @version
> */
> -public class PropertyEditorTypeConverter implements TypeConverter {
> +public class PropertyEditorTypeConverter implements TypeConverter, Service {
>
> private static final Logger LOG = LoggerFactory.getLogger(PropertyEditorTypeConverter.class);
> // use a soft bound cache to avoid using too much memory in case a lot of different classes
> @@ -113,4 +114,15 @@ public class PropertyEditorTypeConverter
> public<T> T mandatoryConvertTo(Class<T> type, Exchange exchange, Object value) {
> return convertTo(type, value);
> }
> +
> + public void start() throws Exception {
> + // noop
> + }
> +
> + public void stop() throws Exception {
> + // clear caches so we dont leak
> + cache.clear();
> + misses.clear();
> + }
> +
> }
>
>
Re: Re: svn commit: r1164515 - /camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
Posted by Claus Ibsen <cl...@gmail.com>.
Okay I got a bit upset here.
But I cant understand why you want to turn the blind eye to the fact
you introduced a leak, despite being careful.
What is the problem with that class being a service, which then have
callbacks (start/stop) where it can ensure
the cache is cleared when stopping.
What do you want to do instead? Introduce a clear() or reset() method,
and then need to invoke that from the
type converter registry.
You need to do something to ensure the caches is cleared, so we dont leak.
Using service is the preferred way in Camel.
On Fri, Sep 2, 2011 at 5:02 PM, Claus Ibsen <cl...@gmail.com> wrote:
> On Fri, Sep 2, 2011 at 4:50 PM, Hadrian Zbarcea <hz...@gmail.com> wrote:
>> Moving to dev@.
>>
>> -1. Please revert your change and let's fix it properly. In the future
>> please do not revert my changes without discussing that with me, even if
>> you believe it's the right thing to do. The PropertyEditoryTypeConverter
>> and being a Service and leaks are two separate issues.
>>
>
> Fix it properly? Well you introduced bugs in the core in the first place!
> The existing code works fine.
>
> A service is a lifecycle callback that allows to do code in starting
> and stopping.
> And being able to clear the maps when stopping is what the service
> solves for us.
>
> I dont see a point reverting this. In fact if we talk about -1 then we
> should consider
> -1 your commits at first then.
>
>
>
>
>> Hadrian
>>
>>
>> On 09/02/2011 08:51 AM, davsclaus@apache.org wrote:
>>>
>>> Author: davsclaus
>>> Date: Fri Sep 2 12:51:57 2011
>>> New Revision: 1164515
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1164515&view=rev
>>> Log:
>>> CAMEL-4392. Clear cache maps when stopping Camel to avoid leaks
>>>
>>> Modified:
>>>
>>> camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
>>>
>>> Modified:
>>> camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
>>> URL:
>>> http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java?rev=1164515&r1=1164514&r2=1164515&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
>>> (original)
>>> +++
>>> camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
>>> Fri Sep 2 12:51:57 2011
>>> @@ -22,6 +22,7 @@ import java.util.HashMap;
>>> import java.util.Map;
>>>
>>> import org.apache.camel.Exchange;
>>> +import org.apache.camel.Service;
>>> import org.apache.camel.TypeConverter;
>>> import org.apache.camel.util.LRUSoftCache;
>>> import org.apache.camel.util.ObjectHelper;
>>> @@ -34,7 +35,7 @@ import org.slf4j.LoggerFactory;
>>> *
>>> * @version
>>> */
>>> -public class PropertyEditorTypeConverter implements TypeConverter {
>>> +public class PropertyEditorTypeConverter implements TypeConverter,
>>> Service {
>>>
>>> private static final Logger LOG =
>>> LoggerFactory.getLogger(PropertyEditorTypeConverter.class);
>>> // use a soft bound cache to avoid using too much memory in case a
>>> lot of different classes
>>> @@ -113,4 +114,15 @@ public class PropertyEditorTypeConverter
>>> public<T> T mandatoryConvertTo(Class<T> type, Exchange exchange,
>>> Object value) {
>>> return convertTo(type, value);
>>> }
>>> +
>>> + public void start() throws Exception {
>>> + // noop
>>> + }
>>> +
>>> + public void stop() throws Exception {
>>> + // clear caches so we dont leak
>>> + cache.clear();
>>> + misses.clear();
>>> + }
>>> +
>>> }
>>>
>>>
>>
>
>
>
> --
> Claus Ibsen
> -----------------
> FuseSource
> Email: cibsen@fusesource.com
> Web: http://fusesource.com
> Twitter: davsclaus, fusenews
> Blog: http://davsclaus.blogspot.com/
> Author of Camel in Action: http://www.manning.com/ibsen/
>
--
Claus Ibsen
-----------------
FuseSource
Email: cibsen@fusesource.com
Web: http://fusesource.com
Twitter: davsclaus, fusenews
Blog: http://davsclaus.blogspot.com/
Author of Camel in Action: http://www.manning.com/ibsen/
Re: Re: svn commit: r1164515 - /camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
Posted by Claus Ibsen <cl...@gmail.com>.
On Fri, Sep 2, 2011 at 4:50 PM, Hadrian Zbarcea <hz...@gmail.com> wrote:
> Moving to dev@.
>
> -1. Please revert your change and let's fix it properly. In the future
> please do not revert my changes without discussing that with me, even if
> you believe it's the right thing to do. The PropertyEditoryTypeConverter
> and being a Service and leaks are two separate issues.
>
Fix it properly? Well you introduced bugs in the core in the first place!
The existing code works fine.
A service is a lifecycle callback that allows to do code in starting
and stopping.
And being able to clear the maps when stopping is what the service
solves for us.
I dont see a point reverting this. In fact if we talk about -1 then we
should consider
-1 your commits at first then.
> Hadrian
>
>
> On 09/02/2011 08:51 AM, davsclaus@apache.org wrote:
>>
>> Author: davsclaus
>> Date: Fri Sep 2 12:51:57 2011
>> New Revision: 1164515
>>
>> URL: http://svn.apache.org/viewvc?rev=1164515&view=rev
>> Log:
>> CAMEL-4392. Clear cache maps when stopping Camel to avoid leaks
>>
>> Modified:
>>
>> camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
>>
>> Modified:
>> camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
>> URL:
>> http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java?rev=1164515&r1=1164514&r2=1164515&view=diff
>>
>> ==============================================================================
>> ---
>> camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
>> (original)
>> +++
>> camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
>> Fri Sep 2 12:51:57 2011
>> @@ -22,6 +22,7 @@ import java.util.HashMap;
>> import java.util.Map;
>>
>> import org.apache.camel.Exchange;
>> +import org.apache.camel.Service;
>> import org.apache.camel.TypeConverter;
>> import org.apache.camel.util.LRUSoftCache;
>> import org.apache.camel.util.ObjectHelper;
>> @@ -34,7 +35,7 @@ import org.slf4j.LoggerFactory;
>> *
>> * @version
>> */
>> -public class PropertyEditorTypeConverter implements TypeConverter {
>> +public class PropertyEditorTypeConverter implements TypeConverter,
>> Service {
>>
>> private static final Logger LOG =
>> LoggerFactory.getLogger(PropertyEditorTypeConverter.class);
>> // use a soft bound cache to avoid using too much memory in case a
>> lot of different classes
>> @@ -113,4 +114,15 @@ public class PropertyEditorTypeConverter
>> public<T> T mandatoryConvertTo(Class<T> type, Exchange exchange,
>> Object value) {
>> return convertTo(type, value);
>> }
>> +
>> + public void start() throws Exception {
>> + // noop
>> + }
>> +
>> + public void stop() throws Exception {
>> + // clear caches so we dont leak
>> + cache.clear();
>> + misses.clear();
>> + }
>> +
>> }
>>
>>
>
--
Claus Ibsen
-----------------
FuseSource
Email: cibsen@fusesource.com
Web: http://fusesource.com
Twitter: davsclaus, fusenews
Blog: http://davsclaus.blogspot.com/
Author of Camel in Action: http://www.manning.com/ibsen/