lppedd/RPG

CEE0810 Error on Reallocate Statement

acejoh opened this issue · 7 comments

Hi,

Thanks for sharing your code. I ran into an issue when adding a 'large' amount of entries to my hashmap. After a while I would get a CEE0810 Error on a hashmapPut call.
After some testing I discovered that the problem lies in the addEntry procedure:

   if (entryPtr = *null);
       allocSpace(entryPtr:ENTRY_SIZE);
       bucket.entryPtr = entryPtr;
   ...
     allocSpace(entry.keyPtr:keySize);
     allocSpace(entry.valuePtr:valueSize); 

Space is allocated but not initialized. As a result entry.keyPtr and entry.valuePtr could contain garbage and this causes allocSpace to fail with the CEE0810 error.
My solution was to add a "clear entry" statement after allocSpace(entryPtr:ENTRY_SIZE);

The same problem applies to arraylist: (arrayAdd procedure)

    entryPtr = getEntry(arrayPtr:index_);
     clear entry;                                                     // initialize entry
     allocSpace(entry.objectPtr:objectSize); 

Kind regards

Hi @acejoh! Thanks for sharing the problem.
It has been a while since I last touched RPG, but I'll take a look at it.
How many entries did you add? (even tought this will be random)

As you said the problem relies here, in allocSpace, because of the pointer field being dirty and not null, the %realloc branch is choosen.

if (ptr = *null);
  ptr = %alloc(bytes);
else;
  ptr = %realloc(ptr:bytes);
endif;

I'm thinking of a way to centralize the solution in here, and not for each procedure, as we could forget if we add other operations.

Btw, it's nice to see someone using this code. I hope I coded it well

I'm not exactly sure how many, probably in the hundreds, not thousands. Interestingly, it was (or seemed) deterministic: my program would crash on the same index, every time.
I can't think of a clean way to centralize the solution in the allocSpace procedure. I think it should be left short and sweet, as it is now. The arrayAdd and addEntry procedures already are quite centralized.
Your code is well written, no worries there ;)

@acejoh We can do as you proposed, or we can MONITOR -> ON-ERROR in allocSpace and go with %alloc.

I didn't think of that, that's a good idea. Could potentially hide other real errors though?
But maybe it's better to go with monitor as I've ran into the same error a few more times when repeatedly calling the program in the same job.
A potential candidate for this new error is in the HashNew and ArrayNew procedures, these also allocate space without initializing. I've added a clear and will keep you notified.

@acejoh We can MONITOR specifically for this error, and let others surface.
Basically potential candidates are the ones that have children (inner) pointer fields.
Should we worry about performance? I think after a certain amount of space used this error will be recurrent.

Btw, you could just set them to *null prior to calling allocSpace. No need to clear.

Ofcourse, I forget about the ability to monitor for specific messages. In that case I think it's a good idea to implement both solutions (initializing newly allocated data and monitoring for errors), for me personally stability takes precedence over performance in this case. If you want I can try to do some performance tests though?
Thanks for your assistance so far 👍
edit: I think it's good practice to initialize all newly allocated memory, by using clear. Any reason why we should just set the pointer to null?

@acejoh It's nothing : )
Yeah sure! If you have some spare time to verify performance go for it.

Well those fields contain addresses that will be overridden by the %alloc call. clear would be called for nothing.