Changes between Initial Version and Version 1 of Ticket #13447, comment 51


Ignore:
Timestamp:
09/18/12 17:23:09 (7 years ago)
Author:
nbruin
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #13447, comment 51

    initial v1  
    6060}}}
    6161which is in step with our use of the ref field. But the code is commented out! There is such a thing as an `intrusive_ptr` in Boost and I think it needs exactly those two functions declared to function. If this is what is intended then we're OK. If this is commented out because now a different scheme is used, we may be in trouble.
     62
     63'''EDIT:''' `kernel/Number.h` contains uncommented definitions:
     64{{{#!C++
     65using namespace boost;
     66inline void intrusive_ptr_add_ref(ring r){
     67    r->ref++;
     68    //Print("ref count after add: %d", r->ref);
     69}
     70inline void intrusive_ptr_release(ring r){
     71    if (r->ref<=0) rDelete(r);
     72    else {
     73    r->ref--;
     74
     75    }
     76    //Print("ref count after release: %d", r->ref);
     77}
     78}}}
     79I find the `ptr_release` a little worrisome: Do the singular people believe that `r->ref ==0` means that there is still a reference? Do they really want to count references "-1"-based? I guess one can count it as "additional" references. I'm afraid they might. From
     80`Singular/ipshell.cc`:
     81{{{#!C++
     82void rKill(ring r)
     83{
     84  if ((r->ref<=0)&&(r->order!=NULL))
     85  {
     86...
     87    rDelete(r);
     88    return;
     89  }
     90  r->ref--;
     91}
     92
     93void rKill(idhdl h)
     94{
     95  ring r = IDRING(h);
     96  int ref=0;
     97  if (r!=NULL)
     98  {
     99    ref=r->ref;
     100    rKill(r);
     101  }
     102  if (h==currRingHdl)
     103  {
     104    if (ref<=0) { currRing=NULL; currRingHdl=NULL;}
     105    else
     106    {
     107      currRingHdl=rFindHdl(r,currRingHdl,NULL);
     108    }
     109  }
     110}
     111}}}
     112this code all indicates that special action is required only if `ref<=0` 'before' decreasing. Can you check with singular people if that's the correct usage of the `ref` field? If we use it in a different way I image nasty bugs could arise later (our use would prevent singular from ever deleting a ring we've had our hands on (if it would ever do that) and if we get a ring that singular initially constructed, we could delete it prematurely, due to an off-by-one.
     113
     114In that case we should probably ensure that `singular_ring_ref` and `singular_delete_ring` are somehow aliased to `boost::intrusive_ptr_add_ref` and `intrusive_ptr_release` from `kernel/Number.h`.