You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by Zoran Regvart <zo...@regvart.com> on 2017/03/15 08:49:16 UTC
I've made Service extend from Closeable
Hi,
just giving the discussion on the PR#1537[1], CAMEL-11011[2] more
visibility here if anyone would like to comment.
The downside I found was that the IDE issues a warning about
'Potential resource leak'. I don't know if this would be reason enough
to revert this?
zoran
[1] https://github.com/apache/camel/pull/1537
[2] https://issues.apache.org/jira/browse/CAMEL-11011
--
Zoran Regvart
Re: I've made Service extend from Closeable
Posted by Zoran Regvart <zo...@regvart.com>.
I'm going to revert this, Sonar would flag this also as a bug (S:2095 [1])
zoran
[1] https://htmlpreview.github.io/?https://github.com/SonarSource/sonar-java/blob/master/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/S2095_java.html
--
Zoran Regvart
Re: I've made Service extend from Closeable
Posted by Zoran Regvart <zo...@regvart.com>.
Hi Claus,
On Fri, Mar 17, 2017 at 9:09 AM, Claus Ibsen <cl...@gmail.com> wrote:
> On Thu, Mar 16, 2017 at 11:51 PM, Zoran Regvart <zo...@regvart.com> wrote:
>> Yeah,
>> I'm only worried that this would introduce confusion amongst users,
>> and once we release a version of Camel with this it's forever there.
>>
>
> Yeah we could revert it.
I'm leaning on doing that, and as a side note I should have based that
on java.lang.AutoCloseable not on java.io.Closeable, that would make
more sense
> Or what if you add closable on ServiceSupport which they all extend,
> would it work with try with resources still, and then you can
> implement the close method directly on ServiceSupport which maybe
> would not cause Eclipse to show that problem?
doesn't help, I think it tracks if the call to close() is from user
code, which is makes sense if it doesn't look at the call graph, so if
I move the logic in stop() into close() and make stop() delegate to
close() I get the same warning, and from what I can read in the
Eclipse docs[1] this is not the case today
> If not we can log a ticket about an API change for Camel 3.0. And look
> at this then.
perhaps for 3.0 would make sense to switch close() and stop(), or
perhaps in that time tools will look at the call graph
I wonder what other tools like Sonar think about this, I'll try to check that...
zoran
[1] http://help.eclipse.org/neon/index.jsp?topic=%2Forg.eclipse.jdt.doc.user%2Ftasks%2Ftask-avoiding_resource_leaks.htm
--
Zoran Regvart
Re: I've made Service extend from Closeable
Posted by Claus Ibsen <cl...@gmail.com>.
On Thu, Mar 16, 2017 at 11:51 PM, Zoran Regvart <zo...@regvart.com> wrote:
> Yeah,
> I'm only worried that this would introduce confusion amongst users,
> and once we release a version of Camel with this it's forever there.
>
Yeah we could revert it.
Or what if you add closable on ServiceSupport which they all extend,
would it work with try with resources still, and then you can
implement the close method directly on ServiceSupport which maybe
would not cause Eclipse to show that problem?
If not we can log a ticket about an API change for Camel 3.0. And look
at this then.
> zoran
>
> On Thu, Mar 16, 2017 at 3:13 PM, Claus Ibsen <cl...@gmail.com> wrote:
>> Hi
>>
>> Okay se we do not see this in IDEA - its after all a smarter editor ;)
>>
>> On Wed, Mar 15, 2017 at 10:26 AM, Zoran Regvart <zo...@regvart.com> wrote:
>>> Hi,
>>> for instance in BeanLookupUsingJndiRegistryIssueTest.java:34[1], the
>>> warrning I get in Eclipse is:
>>>
>>> "Resource leak: 'camel' is never closed"
>>>
>>> this is because the resource leak detection code in IDE is looking for
>>> close(), but we have stop()
>>>
>>> zoran
>>>
>>> [1] https://github.com/apache/camel/blob/master/camel-core/src/test/java/org/apache/camel/component/bean/BeanLookupUsingJndiRegistryIssueTest.java#L34
>>>
>>> On Wed, Mar 15, 2017 at 10:01 AM, Claus Ibsen <cl...@gmail.com> wrote:
>>>> Hi
>>>>
>>>> Is there an example you can point we can open in our IDEs to see ?
>>>>
>>>>
>>>> On Wed, Mar 15, 2017 at 9:49 AM, Zoran Regvart <zo...@regvart.com> wrote:
>>>>> Hi,
>>>>> just giving the discussion on the PR#1537[1], CAMEL-11011[2] more
>>>>> visibility here if anyone would like to comment.
>>>>> The downside I found was that the IDE issues a warning about
>>>>> 'Potential resource leak'. I don't know if this would be reason enough
>>>>> to revert this?
>>>>>
>>>>> zoran
>>>>>
>>>>> [1] https://github.com/apache/camel/pull/1537
>>>>> [2] https://issues.apache.org/jira/browse/CAMEL-11011
>>>>> --
>>>>> Zoran Regvart
>>>>
>>>>
>>>>
>>>> --
>>>> Claus Ibsen
>>>> -----------------
>>>> http://davsclaus.com @davsclaus
>>>> Camel in Action 2: https://www.manning.com/ibsen2
>>>
>>>
>>>
>>> --
>>> Zoran Regvart
>>
>>
>>
>> --
>> Claus Ibsen
>> -----------------
>> http://davsclaus.com @davsclaus
>> Camel in Action 2: https://www.manning.com/ibsen2
>
>
>
> --
> Zoran Regvart
--
Claus Ibsen
-----------------
http://davsclaus.com @davsclaus
Camel in Action 2: https://www.manning.com/ibsen2
Re: I've made Service extend from Closeable
Posted by Zoran Regvart <zo...@regvart.com>.
Yeah,
I'm only worried that this would introduce confusion amongst users,
and once we release a version of Camel with this it's forever there.
zoran
On Thu, Mar 16, 2017 at 3:13 PM, Claus Ibsen <cl...@gmail.com> wrote:
> Hi
>
> Okay se we do not see this in IDEA - its after all a smarter editor ;)
>
> On Wed, Mar 15, 2017 at 10:26 AM, Zoran Regvart <zo...@regvart.com> wrote:
>> Hi,
>> for instance in BeanLookupUsingJndiRegistryIssueTest.java:34[1], the
>> warrning I get in Eclipse is:
>>
>> "Resource leak: 'camel' is never closed"
>>
>> this is because the resource leak detection code in IDE is looking for
>> close(), but we have stop()
>>
>> zoran
>>
>> [1] https://github.com/apache/camel/blob/master/camel-core/src/test/java/org/apache/camel/component/bean/BeanLookupUsingJndiRegistryIssueTest.java#L34
>>
>> On Wed, Mar 15, 2017 at 10:01 AM, Claus Ibsen <cl...@gmail.com> wrote:
>>> Hi
>>>
>>> Is there an example you can point we can open in our IDEs to see ?
>>>
>>>
>>> On Wed, Mar 15, 2017 at 9:49 AM, Zoran Regvart <zo...@regvart.com> wrote:
>>>> Hi,
>>>> just giving the discussion on the PR#1537[1], CAMEL-11011[2] more
>>>> visibility here if anyone would like to comment.
>>>> The downside I found was that the IDE issues a warning about
>>>> 'Potential resource leak'. I don't know if this would be reason enough
>>>> to revert this?
>>>>
>>>> zoran
>>>>
>>>> [1] https://github.com/apache/camel/pull/1537
>>>> [2] https://issues.apache.org/jira/browse/CAMEL-11011
>>>> --
>>>> Zoran Regvart
>>>
>>>
>>>
>>> --
>>> Claus Ibsen
>>> -----------------
>>> http://davsclaus.com @davsclaus
>>> Camel in Action 2: https://www.manning.com/ibsen2
>>
>>
>>
>> --
>> Zoran Regvart
>
>
>
> --
> Claus Ibsen
> -----------------
> http://davsclaus.com @davsclaus
> Camel in Action 2: https://www.manning.com/ibsen2
--
Zoran Regvart
Re: I've made Service extend from Closeable
Posted by Claus Ibsen <cl...@gmail.com>.
Hi
Okay se we do not see this in IDEA - its after all a smarter editor ;)
On Wed, Mar 15, 2017 at 10:26 AM, Zoran Regvart <zo...@regvart.com> wrote:
> Hi,
> for instance in BeanLookupUsingJndiRegistryIssueTest.java:34[1], the
> warrning I get in Eclipse is:
>
> "Resource leak: 'camel' is never closed"
>
> this is because the resource leak detection code in IDE is looking for
> close(), but we have stop()
>
> zoran
>
> [1] https://github.com/apache/camel/blob/master/camel-core/src/test/java/org/apache/camel/component/bean/BeanLookupUsingJndiRegistryIssueTest.java#L34
>
> On Wed, Mar 15, 2017 at 10:01 AM, Claus Ibsen <cl...@gmail.com> wrote:
>> Hi
>>
>> Is there an example you can point we can open in our IDEs to see ?
>>
>>
>> On Wed, Mar 15, 2017 at 9:49 AM, Zoran Regvart <zo...@regvart.com> wrote:
>>> Hi,
>>> just giving the discussion on the PR#1537[1], CAMEL-11011[2] more
>>> visibility here if anyone would like to comment.
>>> The downside I found was that the IDE issues a warning about
>>> 'Potential resource leak'. I don't know if this would be reason enough
>>> to revert this?
>>>
>>> zoran
>>>
>>> [1] https://github.com/apache/camel/pull/1537
>>> [2] https://issues.apache.org/jira/browse/CAMEL-11011
>>> --
>>> Zoran Regvart
>>
>>
>>
>> --
>> Claus Ibsen
>> -----------------
>> http://davsclaus.com @davsclaus
>> Camel in Action 2: https://www.manning.com/ibsen2
>
>
>
> --
> Zoran Regvart
--
Claus Ibsen
-----------------
http://davsclaus.com @davsclaus
Camel in Action 2: https://www.manning.com/ibsen2
Re: I've made Service extend from Closeable
Posted by Zoran Regvart <zo...@regvart.com>.
Hi,
for instance in BeanLookupUsingJndiRegistryIssueTest.java:34[1], the
warrning I get in Eclipse is:
"Resource leak: 'camel' is never closed"
this is because the resource leak detection code in IDE is looking for
close(), but we have stop()
zoran
[1] https://github.com/apache/camel/blob/master/camel-core/src/test/java/org/apache/camel/component/bean/BeanLookupUsingJndiRegistryIssueTest.java#L34
On Wed, Mar 15, 2017 at 10:01 AM, Claus Ibsen <cl...@gmail.com> wrote:
> Hi
>
> Is there an example you can point we can open in our IDEs to see ?
>
>
> On Wed, Mar 15, 2017 at 9:49 AM, Zoran Regvart <zo...@regvart.com> wrote:
>> Hi,
>> just giving the discussion on the PR#1537[1], CAMEL-11011[2] more
>> visibility here if anyone would like to comment.
>> The downside I found was that the IDE issues a warning about
>> 'Potential resource leak'. I don't know if this would be reason enough
>> to revert this?
>>
>> zoran
>>
>> [1] https://github.com/apache/camel/pull/1537
>> [2] https://issues.apache.org/jira/browse/CAMEL-11011
>> --
>> Zoran Regvart
>
>
>
> --
> Claus Ibsen
> -----------------
> http://davsclaus.com @davsclaus
> Camel in Action 2: https://www.manning.com/ibsen2
--
Zoran Regvart
Re: I've made Service extend from Closeable
Posted by Claus Ibsen <cl...@gmail.com>.
Hi
Is there an example you can point we can open in our IDEs to see ?
On Wed, Mar 15, 2017 at 9:49 AM, Zoran Regvart <zo...@regvart.com> wrote:
> Hi,
> just giving the discussion on the PR#1537[1], CAMEL-11011[2] more
> visibility here if anyone would like to comment.
> The downside I found was that the IDE issues a warning about
> 'Potential resource leak'. I don't know if this would be reason enough
> to revert this?
>
> zoran
>
> [1] https://github.com/apache/camel/pull/1537
> [2] https://issues.apache.org/jira/browse/CAMEL-11011
> --
> Zoran Regvart
--
Claus Ibsen
-----------------
http://davsclaus.com @davsclaus
Camel in Action 2: https://www.manning.com/ibsen2