cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aki Yoshida <elak...@googlemail.com>
Subject Re: org.apache.cxf.endpoint.EndpointImpl may not be used as a hash map key
Date Thu, 19 May 2011 16:35:43 GMT
Thanks for your advice.
Yes, the equals method is already using the identity check.

For RMManager, RMUtils has a utility method getEndpointIdentifier to
return the the identify of a given Endpoint as a string. This string
is basically constructed out of the endpoint info object. This can be
used as the key. But calling this method creates a new string, so I
think we can rather stay with the fixed Endpoint or try to see if
EndpointInfo can be used as the key.

Regards, aki


2011/5/18 Daniel Kulp <dkulp@apache.org>:
> 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