You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by Claude Warren <cl...@xenei.com> on 2015/01/24 12:01:27 UTC
Strange close() behaviour
Background:
1. TDBGraph implements org.apache.jena.atlas.lib.Closable which defines
a close() method
2. Graph does not.
3. DatasetGraphMaker.close() calls close() on the defaultGraph.
The security system creates dynamic proxies.
When a TDBGraph is closed the method that is being attempted is
Closable.close() not Graph.close() (probably has to do with the order they
are defined in the TDBGraph class definition.
The security system sees that it is proxying a method with the same
signature (name and args). but when it attempts to execute the
Closable.close() method and fails since the proxy doesn't really proxy that
method it proxies Graph.close().
I can fix this in the proxy code by calling the Graph.close() rather than
the Closable.close() but I began to wonder if Graph (and other classes with
close() methods) shouldn't extend Closable?
Thoughts?
--
I like: Like Like - The likeliest place on the web
LinkedIn: http://www.linkedin.com/in/claudewarren
Re: Strange close() behaviour
Posted by Claude Warren <cl...@xenei.com>.
Removing closable from TDBGraph? No.
Removing closable from the proxy? No, It proxies all the interfaces from
the original object so that the contracts of the original object are not
broken.
I changed the proxy so that if a method with the same signature is found it
calls the one on the secured object (so in this case it calls
securedGraph.close() which just calls the close on the TDBGraph object it
is wrapping). Other methods have security checks on them.
Claude
--
I like: Like Like - The likeliest place on the web
<http://like-like.xenei.com>
LinkedIn: http://www.linkedin.com/in/claudewarren
Re: Strange close() behaviour
Posted by Andy Seaborne <an...@apache.org>.
On 24/01/15 11:01, Claude Warren wrote:
> Background:
>
> 1. TDBGraph implements org.apache.jena.atlas.lib.Closable which defines
> a close() method
> 2. Graph does not.
> 3. DatasetGraphMaker.close() calls close() on the defaultGraph.
>
>
> The security system creates dynamic proxies.
> When a TDBGraph is closed the method that is being attempted is
> Closable.close() not Graph.close() (probably has to do with the order they
> are defined in the TDBGraph class definition.
>
> The security system sees that it is proxying a method with the same
> signature (name and args). but when it attempts to execute the
> Closable.close() method and fails since the proxy doesn't really proxy that
> method it proxies Graph.close().
>
> I can fix this in the proxy code by calling the Graph.close() rather than
> the Closable.close() but I began to wonder if Graph (and other classes with
> close() methods) shouldn't extend Closable?
>
> Thoughts?
>
What actually happens?
GraphTDB.close() calls super.close()
super is GraphView
GraphView extends GraphBase
so that is Graph.close.
What are you encountering?
GraphTDB.close is both. It's a method name that satisfies
GraphBase.close and Closable.close
Andy
>>
>> The security system sees that it is proxying a method with the same
>> signature (name and args). but when it attempts to execute the
>> Closable.close() method and fails since the proxy doesn't really
proxy that
>> method it proxies Graph.close().
>>
>> I can fix this in the proxy code by calling the Graph.close() rather
than
>> the Closable.close() but I began to wonder if Graph (and other
classes with
>> close() methods) shouldn't extend Closable?
>>
>> Thoughts?
Have you tried removing Closable?
Graph already had close before datasets came along.
Andy
>
Re: Strange close() behaviour
Posted by Andy Seaborne <an...@apache.org>.
On 24/01/15 15:36, Ian Emmons wrote:
> +1 -- Moving to Java 7 should include a comprehensive review of which classes should become closable to enable try-with-resources.
What would be really great is if you'd look through the codebase and
make suggestions as to where it makes sense.
(we have had some earlier discussions on the subject)
>
> Ideally, the Jena code base should also use try-with-resources internally wherever that makes sense.
The critical factor is "where it makes sense" such as where the resource
fits the try-with-resources idiom. Many things do not - they are passed
around, not used within a scoped code block.
e.g. QueryExecution - that does fit the paradigm of AutoClosable very
nicely.
Graph does not seem to be to be used in a try-resource fashion.
> Note that in some cases extending AutoCloseable might be preferable. Closeable.close() may only throw IOException, whereas AutoCloseable.close() may throw Exception.
Yes.
(BTW: wrong closeable - org.apache.jena.atlas.lib.Closeable)
Andy
Re: Strange close() behaviour
Posted by Ian Emmons <ia...@emmons.mobi>.
+1 -- Moving to Java 7 should include a comprehensive review of which classes should become closable to enable try-with-resources.
Ideally, the Jena code base should also use try-with-resources internally wherever that makes sense.
Note that in some cases extending AutoCloseable might be preferable. Closeable.close() may only throw IOException, whereas AutoCloseable.close() may throw Exception.
> On Jan 24, 2015, at 6:01 AM, Claude Warren <cl...@xenei.com> wrote:
>
> Background:
>
> 1. TDBGraph implements org.apache.jena.atlas.lib.Closable which defines
> a close() method
> 2. Graph does not.
> 3. DatasetGraphMaker.close() calls close() on the defaultGraph.
>
>
> The security system creates dynamic proxies.
> When a TDBGraph is closed the method that is being attempted is
> Closable.close() not Graph.close() (probably has to do with the order they
> are defined in the TDBGraph class definition.
>
> The security system sees that it is proxying a method with the same
> signature (name and args). but when it attempts to execute the
> Closable.close() method and fails since the proxy doesn't really proxy that
> method it proxies Graph.close().
>
> I can fix this in the proxy code by calling the Graph.close() rather than
> the Closable.close() but I began to wonder if Graph (and other classes with
> close() methods) shouldn't extend Closable?
>
> Thoughts?