[vw7.7] Opentalk.OtWeakKeyDictionary bug?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[vw7.7] Opentalk.OtWeakKeyDictionary bug?

mkobetic-2
OK, here goes a bit more info about the associated ARs and fixes. the patch is attached (also compressed in case some spam filters have .st alergies). These were one of the very first fixes in OpentalkBase so should be directly applicable to 7.7

58426: "More concerns about WeakKeyDictionary"
58838: "OtWeak?Dictionary sporadically "loses" its entries for brief periods of times (usually under load)"

* made sure that reclaimElements is always followed by either grow, shrink or rehash on every execution path at every call-site (finalizeElements, grow)
* fixed includesKey: which was completely broken
* moved accessLock setup from makeDependent to initSize:, otherwise we break the lock every time we grow or shrink
* added keysDo: because it is used by DictionaryInspector and so without it you cannot inspect these dictionaries
* extended OtETimeout to capture the request and transport as well
* fixed OtWeakKeyDictionary>>reclaimElements to reclaim both keys and values
* fixed OtW!
eakKeyDictionary>>atIndex:putKey:andValue: to look at key not value when updating tally
* OtWeakKeyDictionary>>at:ifAbsentPut: to only grow if actually adding an element
* minor optimization in OtWeakKeyDictionary>>at:put:

[hidden email] wrote:
> We've had several reports of the weak dictionary not keeping tally in sync with the contents. This should be fixed in 7.8 builds (OpentalkBase(7.8 - 2)). I'll extract the changes as a patch and post it here shortly.
>
> "Mark Plas"<[hidden email]> wrote:
> > Since using VW7.7, we sometimes encounter an endless loop in OtWeakKeyDictionary.

....
_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

100302-OtWeakDictionary-fixes.st (11K) Download Attachment
100302-OtWeakDictionary-fixes.st.gz (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [vw7.7] Opentalk.OtWeakKeyDictionary bug?

Mike Hales
I've been wrestling with some of the same problems, and am glad to see this patch. One more thing I noticed when I was debugging some of these issues is in:

RequestTransport>>newRequestId
    "We hope that a request would not last long
    enough to permit the reuse of an id number."

    ^nextRequestId := (nextRequestId + 1) \\ 536870911

If nextRequestId does reach 536870910 (ie. one shy of SmallInteger maxValue) when called again the nextRequestId will be zero, which is invalid as a key in an OtWeakKeyDictionary where this value is used, so this method should probably be:

newRequestId
    "We hope that a request would not last long
    enough to permit the reuse of an id number."

    ^nextRequestId := (nextRequestId + 1) \\ 536870911 max: 1

That's quite a few requests, so the rollover may have never been tested.

Mike


Mike Hales
Engineering Manager
KnowledgeScape
www.kscape.com


On Tue, Mar 2, 2010 at 7:31 AM, <[hidden email]> wrote:
OK, here goes a bit more info about the associated ARs and fixes. the patch is attached (also compressed in case some spam filters have .st alergies). These were one of the very first fixes in OpentalkBase so should be directly applicable to 7.7

58426: "More concerns about WeakKeyDictionary"
58838: "OtWeak?Dictionary sporadically "loses" its entries for brief periods of times (usually under load)"

* made sure that reclaimElements is always followed by either grow, shrink or rehash on every execution path at every call-site (finalizeElements, grow)
* fixed includesKey: which was completely broken
* moved accessLock setup from makeDependent to initSize:, otherwise we break the lock every time we grow or shrink
* added keysDo: because it is used by DictionaryInspector and so without it you cannot inspect these dictionaries
* extended OtETimeout to capture the request and transport as well
* fixed OtWeakKeyDictionary>>reclaimElements to reclaim both keys and values
* fixed OtW!
eakKeyDictionary>>atIndex:putKey:andValue: to look at key not value when updating tally
* OtWeakKeyDictionary>>at:ifAbsentPut: to only grow if actually adding an element
* minor optimization in OtWeakKeyDictionary>>at:put:

[hidden email] wrote:
> We've had several reports of the weak dictionary not keeping tally in sync with the contents. This should be fixed in 7.8 builds (OpentalkBase(7.8 - 2)). I'll extract the changes as a patch and post it here shortly.
>
> "Mark Plas"<[hidden email]> wrote:
> > Since using VW7.7, we sometimes encounter an endless loop in OtWeakKeyDictionary.

....


_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc
Reply | Threaded
Open this post in threaded view
|

Re: [vw7.7] Opentalk.OtWeakKeyDictionary bug?

mkobetic-2
In reply to this post by mkobetic-2
AR#59534: "[STST] RequestTransport nextRequestId can roll-over to 0 and blow up in requestRegistry"

Thanks!

Martin

"Mike Hales"<[hidden email]> wrote:

> Date: March 2, 2010 1:28:15 PM
> From: "Mike Hales"<[hidden email]>
> To: [hidden email]
> Cc: "vwnc NC"<[hidden email]>
> Subject: Re: [vw7.7] Opentalk.OtWeakKeyDictionary bug?
>
> I've been wrestling with some of the same problems, and am glad to see this
> patch. One more thing I noticed when I was debugging some of these issues is
> in:
>
> RequestTransport>>newRequestId
>     "We hope that a request would not last long
>     enough to permit the reuse of an id number."
>
>     ^nextRequestId := (nextRequestId + 1) \\ 536870911
>
> If nextRequestId does reach 536870910 (ie. one shy of SmallInteger maxValue)
> when called again the nextRequestId will be zero, which is invalid as a key
> in an OtWeakKeyDictionary where this value is used, so this method should
> probably be:
>
> newRequestId
>     "We hope that a request would not last long
>     enough to permit the reuse of an id number."
>
>     ^nextRequestId := (nextRequestId + 1) \\ 536870911 max: 1
>
> That's quite a few requests, so the rollover may have never been tested.
>
> Mike
>
>
> Mike Hales
> Engineering Manager
> KnowledgeScape
> www.kscape.com
>
>
> On Tue, Mar 2, 2010 at 7:31 AM, <[hidden email]> wrote:
>
> > OK, here goes a bit more info about the associated ARs and fixes. the patch
> > is attached (also compressed in case some spam filters have .st alergies).
> > These were one of the very first fixes in OpentalkBase so should be directly
> > applicable to 7.7
> >
> > 58426: "More concerns about WeakKeyDictionary"
> > 58838: "OtWeak?Dictionary sporadically "loses" its entries for brief
> > periods of times (usually under load)"
> >
> > * made sure that reclaimElements is always followed by either grow, shrink
> > or rehash on every execution path at every call-site (finalizeElements,
> > grow)
> > * fixed includesKey: which was completely broken
> > * moved accessLock setup from makeDependent to initSize:, otherwise we
> > break the lock every time we grow or shrink
> > * added keysDo: because it is used by DictionaryInspector and so without it
> > you cannot inspect these dictionaries
> > * extended OtETimeout to capture the request and transport as well
> > * fixed OtWeakKeyDictionary>>reclaimElements to reclaim both keys and
> > values
> > * fixed OtW!
> > eakKeyDictionary>>atIndex:putKey:andValue: to look at key not value when
> > updating tally
> > * OtWeakKeyDictionary>>at:ifAbsentPut: to only grow if actually adding an
> > element
> > * minor optimization in OtWeakKeyDictionary>>at:put:
> >
> > [hidden email] wrote:
> > > We've had several reports of the weak dictionary not keeping tally in
> > sync with the contents. This should be fixed in 7.8 builds (OpentalkBase(7.8
> > - 2)). I'll extract the changes as a patch and post it here shortly.
> > >
> > > "Mark Plas"<[hidden email]> wrote:
> > > > Since using VW7.7, we sometimes encounter an endless loop in
> > OtWeakKeyDictionary.
> >
> > ....

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc