OpenEuphoria

Re: EU 3.0.1 -- there's probably a bug in there somewhere

• Posted by Robert Craig <rds at RapidEuphoria.com> Feb 03, 2007
• 682 views

Robert Craig wrote:
> Pete Lomax wrote:
> > Matt Lewis wrote:
> > > Here's some simpler code that causes a crash.
> > Confirmed here too, on 3.0.0 but not 2.4, 2.5, or cb-3.0.1.
> > > However, after rebuilding from source, I wasn't getting the crash
> > No way to know if a source mod fixes it then...
> > 
> > I had a quick hack and it appears to be crashing on my system in routine
> > restore_privates,
> > in the code:
> > 
> >         // restore the current task's private data (will always be there)
> > 
> >         block = load_private_block(this_routine, current_task);
> > 
> >         // private vars
> >         sym = this_routine->next;
> >         while (sym != NULL && sym->scope <= S_PRIVATE) {
> >             sym->obj = *block++;
> >             sym = sym->next;
> >         }
> > 
> > it crashes on the sym->obj = *block statement. A chunk of heap ends at
> > #004D5FFF
> > and when it crashes block is #004D6000. sym is correctly pointing at the
> > parameter
> > r. I have not studied load/save_private_block, but that is where my
> > suspicions
> > would lie. A random thought I had is that if it were saving and loading
> > everything
> > out-by-one, it might well appear to work fine in 99.9% of cases. Or maybe
> > not.
> 
> Thanks to everyone for helping pin this down.
> I'll be able to look at this later today (in a few hours).
> As you suggest, it could be that I'm allocating slightly less memory 
> than I need in some cases. That would cause occasional crashes
> that might go away with just a trivial change in the user's program.

I wasn't able to duplicate the crash, but I've found a bug 
in load_private_block() that could very well be the cause.

static object *load_private_block(symtab_ptr routine, int task)
// Retrieve a private block and remove it from the list for this routine.
// We know that the block will be there, often near the start of the list.
{
    struct private_block *p;
    struct private_block *prev_p;
    struct private_block *defunct;
    object *block;

    p = routine->u.subp.saved_privates; // won't be NULL
    prev_p = NULL;

    while (TRUE) {
        if (p->task_number == task) {
            block = (object *)&(p->block);

            // unlink it
            if (prev_p == NULL) {
                routine->u.subp.saved_privates = p->next;
            }
            else {
                prev_p->next = p->next;
            }

            EFree(p); // BUG: too soon!!!
            return block;
        }
        prev_p = p;
        p = p->next;
    }
}

In the C code above, I've made the mistake of freeing block p, 
*then* returning a pointer into it. The caller uses that pointer
to retrieve some data. EFree() would usually just put p onto a 
list, but if the storage cache is getting too full (over 2000 blocks), 
it will actually call free(), which, on Windows, could occasionally
result in modification of the freed block, perhaps merging it with 
an adjacent block, shrinking the heap or something.

(Originally, "block" was a separately allocated block of memory, 
but for efficiency, I combined it into block p using a C coding trick.)

The fix is easy since load_private_block() is only called
from one place. I just have to retrieve the data from block p,
*before* calling EFree().

I'll check the new code in tomorrow.
Maybe we need 3.0.2 soon.

Regards,
   Rob Craig
   Rapid Deployment Software
   http://www.RapidEuphoria.com