cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Kulp <dk...@apache.org>
Subject Re: org.apache.cxf.endpoint.EndpointImpl may not be used as a hash map key
Date Wed, 18 May 2011 15:58:39 GMT
On Tuesday, May 17, 2011 10:23:29 AM Aki Yoshida wrote:
> Hi CXF-developers,
> I haven't seen feedbacks to my question that I posted here last week.
> And I am afraid that my message was probably too long. So, I am adding
> a short intro to this issue. (or let me know if should have used the
> users list).
> 
> The above Endpoint implementation uses its super class's HashMap's
> hashCode(). So, when an Endpoint object is stored as a key in HashMap
> or HashSet, you cannot find it in some cases (i.e., when the hashCode
> changes after the key is placed in the table).
> 
> This problem is causing an issue in looking up a valid source sequence
> in WS-RM in some cases.
> 
> My question is, could we change EndpointImpl to implement its own
> hashCode based on its endpoint reference info value so that its
> hashCode value remains table and this object can be used as a key?

Yes.  That makes complete sense.  Likely the equals needs  to be adjusted as 
well to just do == style matching..

> Alternatively, should we simply refrain from using Endpoint as a key?

Ideally, we should try this as well, but with RM, I'm not sure how possible it 
is.    Keeping them in maps has a general affect of making them not garbage 
collectable and, if care isn't properly taken, leaks memory.   At one point, 
we used the Endpoint as keys in a bunch of places, but most of those I was 
able to move the "value" that was associated with the Endpoint into an 
endpoint property itself so we didn't need to store the Endpoint.  

Dan


> I am not familiar with the original usage intention of this Endpoint
> object and I would like to hear your comments before creating a jira
> ticket to fix the mentioned WS-RM issue.

> 
> For details, please refer to the below message, sorry for its length.
> 
> Thanks.
> regards, aki
> 
> 2011/5/13 Aki Yoshida <elakito@googlemail.com>:
> > Hi,
> > While analyzing a problem in WS-RM, which I describe later, I
> > discovered this problem. I will mention it on this dev@cxf list
> > because potentially other components may be affected as well depending
> > on how they are using the instances of this class.
> > 
> > This EndpointImpl class extends AbstractAttributedInterceptorProvider
> > and this class in turn extends HashMap<String, Object>.
> > AbstractAttributedInterceptorProvider overwrites its hashCode() but it
> > simply calls its super class HashMap's hashCode().
> > 
> > Consequently, calling EndpointImpl's hashCode() will call HashMap's
> > hashCode() which will return a different value every time some new
> > items are inserted in the map.
> > 
> > So when you have something like
> > 
> > Endpoint endpoint = ...;
> > map = new HashMap<...>();
> > map.put(endpoint, "A");
> > map.get(endpoint) will still return "A" as long as no one has added
> > something to the endpoint.
> > 
> > But, for example, if you have something like
> > 
> > endpoint.put(USING_ADDRESSING, b);
> > 
> > which incidentally MAPAggregator does, you can no longer find the
> > endpoint in the map (i.e., map.get(endpoint) returns null even though
> > map.keySet().iterator().next().equals(endpoint) returns true).
> > 
> > This problem is causing the WS-RM runtime client to not being able to
> > find the existing endpoints in the reliableEndpoints map held by the
> > RMManager in some situation. In a normal situation, an endpoint is
> > placed in this map in RMOutInterceptor, which comes after
> > MAPAggregator. Consequently, no one manipulates the endpoint before a
> > lookup occurs and you don't observe this problem. However, in a client
> > restarting situation, the client recovers the endpoints during its
> > startup and places them in this map. When another call is sent to one
> > of these endpoints, MAPAggregator is invoked and this changes the
> > endpoint's hashCode value, invalidating the map and we can no longer
> > find this endpoint at RMOutInterceptor. (note that the current
> > ClientPersistenceTest in ws-rm systest does not cover this case).
> > 
> > So, if we want to allow EndpointImpl to be used as a map key, we
> > should overwrite its hashCode() to use its endpointInfo's hash value
> > so that its key value remains stable. I prefer this solution. But I am
> > not sure if someone intentionally wants to have a different hashCode
> > when some changes are made in the map (i.e., to verify if the endpoint
> > object has been manipulated or not).
> > 
> > If we do not choose this option and forbit EndpointImpl to be used as
> > a map key, we will need to fix RMManager to use a different key object
> > in its reliableEndpoints.
> > 
> > Let me know how you think.
> > 
> > Thanks.
> > Regards, aki

-- 
Daniel Kulp
dkulp@apache.org
http://dankulp.com/blog
Talend - http://www.talend.com

Mime
View raw message