Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CEE0810 Error on Reallocate Statement #6

Open
acejoh opened this issue Jun 25, 2018 · 7 comments
Open

CEE0810 Error on Reallocate Statement #6

acejoh opened this issue Jun 25, 2018 · 7 comments
Labels

Comments

@acejoh
Copy link

acejoh commented Jun 25, 2018

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

@lppedd
Copy link
Owner

lppedd commented Jun 25, 2018

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

@lppedd lppedd added the bug label Jun 25, 2018
@acejoh
Copy link
Author

acejoh commented Jun 26, 2018

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 ;)

@lppedd
Copy link
Owner

lppedd commented Jun 27, 2018

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

@acejoh
Copy link
Author

acejoh commented Jul 4, 2018

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.

@lppedd
Copy link
Owner

lppedd commented Jul 4, 2018

@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.

@acejoh
Copy link
Author

acejoh commented Jul 5, 2018

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?

@lppedd
Copy link
Owner

lppedd commented Jul 5, 2018

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants