14886 - new bead maps must addref beads put into map

Status: VERIFIED FIXED

(MailNews Core :: Database, defect, P3)

Description

[davidmc]

Need to add refcount usage in new bead maps; fixe functions:

mork_bool
morkBeadMap::AddBead(morkEnv* ev, morkBead* ioBead)
  // the AddBead() boolean return equals ev->Good().
{
  if ( ioBead && ev->Good() )
  {
    morkBead* oldBead = 0; // old key in the map

    mork_bool put = this->Put(ev, &ioBead, /*val*/ (void*) 0,
      /*key*/ &oldBead, /*val*/ (void*) 0, (mork_change**) 0);

    if ( put ) // replaced an existing key?
    {
      if ( oldBead != ioBead ) // new bead was not already in table?
        ioBead->AddStrongRef(ev); // now there's another ref

      if ( oldBead && oldBead != ioBead ) // need to release old node?
        oldBead->CutStrongRef(ev);
    }
    else
      ioBead->AddStrongRef(ev); // another ref if not already in table
  }
  else if ( !ioBead )
    ev->NilPointerError();

  return ev->Good();
}

mork_bool
morkBeadProbeMap::AddBead(morkEnv* ev, morkBead* ioBead)
{
  if ( ioBead && ev->Good() )
  {
    morkBead* bead = 0; // old key in the map

    mork_bool put = this->MapAtPut(ev, &ioBead, /*val*/ (void*) 0,
      /*key*/ &bead, /*val*/ (void*) 0);

    if ( put ) // replaced an existing key?
    {
      if ( bead != ioBead ) // new bead was not already in table?
        ioBead->AddStrongRef(ev); // now there's another ref

      if ( bead && bead != ioBead ) // need to release old node?
        bead->CutStrongRef(ev);
    }
    else
      ioBead->AddStrongRef(ev); // now there's another ref
  }
  else if ( !ioBead )
    ev->NilPointerError();

  return ev->Good();
}

Comment 1

[David :Bienvenu]

we're not going to hold m10 for this, and DavidMc is on vacation today. We'd
still take the fix tomorrow if he can check it in.

Comment 2

[davidmc]

I can check this in today if that's desired; later this afternoon I'll try to
figure out whether folks want me to do this.  Feel free to give me direction.

Comment 3

[David :Bienvenu]

Didn't Chris give approval? (I vaguely remember that he did.) If so, might as
well check it in.

Comment 4

[davidmc]

checked in; watching tinderbox

Comment 5

[davidmc]

fixed; to verify, see code for add and cut strong ref calls;
to run,  turn on the MORK_CONFIG_BEAD_OVER_NODE_MAPS compiler
switch in morkConfig.h (I might have mispelled that switch).

Comment 6

[ppandit]

Cuurent code shows:

mork_bool
170 morkBeadMap::AddBead(morkEnv* ev, morkBead* ioBead)
171   // the AddBead() boolean return equals ev->Good().
172 {
173   if ( ioBead && ev->Good() )
174   {
175     morkBead* oldBead = 0; // old key in the map
176 
177     mork_bool put = this->Put(ev, &ioBead, /*val*/ (void*) 0,
178       /*key*/ &oldBead, /*val*/ (void*) 0, (mork_change**) 0);
179       
180     if ( put ) // replaced an existing key?
181     {
182       if ( oldBead != ioBead ) // new bead was not already in table?
183         ioBead->AddStrongRef(ev); // now there's another ref
184         
185       if ( oldBead && oldBead != ioBead ) // need to release old node?
186         oldBead->CutStrongRef(ev);
187     }
188     else
189       ioBead->AddStrongRef(ev); // another ref if not already in table
190   }
191   else if ( !ioBead )
192     ev->NilPointerError();
193     
194   return ev->Good();
195 }

from morkNode.cpp:
/*public non-poly*/ mork_uses
474 morkNode::AddStrongRef(morkEnv* ev)
475 {
476   mork_uses outUses = 0;
477   if ( this )
478   {
479     if ( this->IsNode() )
480     {
481       mork_uses uses = mNode_Uses;
482       mork_refs refs = mNode_Refs;
483       if ( refs < uses ) // need to fix broken refs/uses relation?
484       { 
485         this->RefsUnderUsesWarning(ev);
486         mNode_Refs = mNode_Uses = refs = uses;
487       }
488       if ( refs < morkNode_kMaxRefCount ) // not too great?
489       {
490         mNode_Refs = ++refs;
491         mNode_Uses = ++uses;
492       }
493       else
494         this->RefsOverflowWarning(ev);
495 
496       outUses = uses;
497     }
498     else
499       this->NonNodeError(ev);
500   }
501   else
502     ev->NilPointerError();
503   return outUses;
504 }

/*public non-poly*/ mork_uses
544 morkNode::CutStrongRef(morkEnv* ev)
545 {
546   mork_refs outRefs = 0;
547   if ( this )
548   {
549     if ( this->IsNode() )
550     {
551       if ( this->cut_use_count(ev) )
552         outRefs = this->CutWeakRef(ev);
553     }
554     else
555       this->NonNodeError(ev);
556   }
557   else
558     ev->NilPointerError();
559   return outRefs;
560 }

Comment 7

[ppandit]

Based on my code inspection, this looks correct. 
Marking as VERIFIED