> -----Original Message-----
> From: [EMAIL PROTECTED]
> [mailto:[EMAIL PROTECTED]]On Behalf Of marc
> fleury
> Sent: Wednesday, June 13, 2001 1:29 PM
> To: [EMAIL PROTECTED]
> Subject: RE: [JBoss-dev] Should CacheKey copy its id on creation?
>
>
> |> 2- you share the same instance of the PrimaryKey, yes, but the field
> |> variables are not the same so that the byte representation should
> |> not be the
> |> same, REGARDLESS of the adress of the key
> |
> |True, the HashMap lookup is correct because of the
> MarshalledObject stuff.
>
> Ok so equals and hashcode seem to be working on serialized "precomputed"
> stuff, the mo and hashcode are both computed at construction time (if my
> memory is correct) which is why I am still confused as to where
> the problem
> is.  If the map lookups do not depend on the id (which we hold onto) then
> where is the problem, please explain the *problem* to me. (I
> understand the
> fact that you change the PK (bad boy bad boy, but it should still work on
> cache, and explain *where* it doesn't work)
>
> |But, in the above example code for Optimized = true,
> |listOfIds.length number
> |of beans all share the same instance of the PrimaryKey passed in to
> |findByPrimaryKey, that is, the CacheKey.id instance AND the
> |EnterpriseContext.id instance!!!  So when the CMP EntityBean
> does a store,
> |it will lookup on a bad PrimaryKey.
>
> ah!!!! ok... yes the store DOES rely on the id... THAT is the problem.
>
> Got you... we hold onto the PrimaryKey and **persistent store** operations
> read the ECC.id which is a hard link to the cachekey.id and thus
> BECAUSE YOU
> PASS THE SAME KEY OVER AND OVER the store uses the new ID.
>
> It is a code bug (you should create new keys not reuse them) but still it
> does f*ck up the container store (the cache still works)... Got it.
>

BTW, it's only a code bug in our application because we didn't know about
the Optimize flag.  The spec states that all calls on home and remote
interfaces must copy their parameters.  The Optimize flag, out-of-the-box,
is True, isn't it?  Maybe "true" is not the correct out-of-the-box default
setting, but that's just my humble opinion.


> |> 3- So no the cache key should not reference the same thing.
> |What is wrong
> |> is that we don't care about the "adress" of the object, just the
> |> values.  If
> |> that is not what is compute in the cache key then that is wrong.
> |>
> |
> |Again, CacheKey.equals DOES work. But the PrimaryKeys are all shared.
>
> Yes, I know, but now I understand the *problem* is the store
> operations that
> rely on the ecc.id (==cachekey.id).  In fact that is a problem for
> *everyone* relying on the cachekey.id since you reuse the PrimaryKey. I
> can't remember who else accesses that.
>
> |> I do object, hold on on that, I am not sure I believe your
> |> analysis at this
> |> point, so I am certainly not going to buy a solution either
> |(even tested),
> |> it is core enough that we need to think about it.  Ping me tomorrow or
> |> friday.
>
> No need to come back on Friday then, you got it.
>
> The analysis (complete) is that anyone relying on the id of the keys can
> shoot itself in the foot when optimizations are on and he changes the keys
> dynamically and passes them to other instances.
>
> While I could argue that it is a client bug, (don't pass keys
> that you don't
> copy and change them), we can in fact protect you and it from
> happening and
> the place is in fact the copy of the cachekey.id.  Good catch.
>
> |If you don't believe it, how come when we turned off Optimized everything
> |worked.  As a different test, we didn't share PrimaryKey instances when
> |calling findByPrimaryKey, and that worked as well!  I'm not crazy here.
>
> No you are not, you just need to explain to me you MF :) in fact
> I will say
> again "Good catch".
>

Sorry for being so obscure before.  Sometimes I don't explain things all
that great.  Thanks for reviewing, though....(We're under the gun here
trying to release our product.)

> |I've been looking at this code for more than a week now trying to
> |figure out
> |why our InstanceCache was all fucked up and finally, finally found the
> |solution.  BTW, the Optimized flag isn't even documented within
> JBoss.....
>
> You got the solution, do get the copy of the id on creation (as your title
> explained).
> As a byproduct I would like you to do 2 things
> 1- commit that patch to the main tree, it is a very good patch, you nailed
> the solution (even if the analysis was obscure to me at first)
> 2- do a grep on WHO uses the cachekey.id either by call either by
> reference
> directly.  And put that list in the code source of the cachekey.  We know
> the store does it for sure, who else? I don't remember to be honest.  The
> solution will work for everyone but just for the record so that
> we know who
> calls what.
>

No need to grep, I'll change cachekey.id --> cacheKey.getId() and let the
compiler figure it out.  CacheKey should be immutable, don't you agree?


Bill



_______________________________________________
Jboss-development mailing list
[EMAIL PROTECTED]
http://lists.sourceforge.net/lists/listinfo/jboss-development

Reply via email to