Recent

Author Topic: Heap memory release error message  (Read 439 times)

Dimitrios Chr. Ioannidis

  • Full Member
  • ***
  • Posts: 167
    • Nephelae
Heap memory release error message
« on: July 02, 2022, 11:08:18 am »
Hi,

  sometimes I get this error message when I release a heap allocated memory pointer :

Code: Pascal  [Select][+][-]
  1. Marked memory at $015F0220 invalid
  2. Wrong signature $CD065852 instead of 01008BD7
  3.  

  The memory is allocated with Getmem(FMemoryData, FSize) and released with Freemem(FMemoryData). The FMemoryData is Pointer type.

  What is the meaning of this error message ?

  FPC: 3.2.2
  OS : Windows 10

regards,
Dimitrios Chr. Ioannidis
http://www.nephelae.eu

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 8095
  • Debugger - SynEdit - and more
    • wiki
Re: Heap memory release error message
« Reply #1 on: July 02, 2022, 01:32:34 pm »
That means you did write data into memory that you had already freed.
Or sometimes into memory you never had. I.e. into the internal data of the mem-manager.

Some possibilities are:
- A dangling pointer (including types with internal pointer)
- An out of range access: MyPtrByte := GetMem(10); MyPtrByte[20] :=

The probable easiest way to find out more is valgrind.
But that requires Linux. If you can compile your app for Linux, and use "valgrind --tool=memcheck"

Dimitrios Chr. Ioannidis

  • Full Member
  • ***
  • Posts: 167
    • Nephelae
Re: Heap memory release error message
« Reply #2 on: July 03, 2022, 06:52:29 pm »
Hi,

That means you did write data into memory that you had already freed.
Or sometimes into memory you never had. I.e. into the internal data of the mem-manager.

thx for the tips.

You helped me finish a lock free single producer / single consumer ring buffer implementation I needed ! ( at least after a lot of tests, I think it is lock free .... :) )

Thank you !

regards,
« Last Edit: July 07, 2022, 12:21:26 pm by Dimitrios Chr. Ioannidis »
Dimitrios Chr. Ioannidis
http://www.nephelae.eu

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 8095
  • Debugger - SynEdit - and more
    • wiki
Re: Heap memory release error message
« Reply #3 on: July 03, 2022, 07:34:48 pm »
Btw valgrind also has 2 thread analysers.

Though they usually look for the correct order of locks and critical sections and access of data from various threads in between. So they are likely of little use to you.
Also FPC causes some false positives, like extra lock-free....

However, if you do have any code that uses locks, they may be worth a look. It is however a bit of work to sift through the results.

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 8095
  • Debugger - SynEdit - and more
    • wiki
Re: Heap memory release error message
« Reply #4 on: July 03, 2022, 08:31:26 pm »
You helped me finish a lock free producer / consumer ring buffer] implementation I needed ! ( at least after a lot of tests, I think it is lock free .... :) )
Well you don't call any "locks", "critical sections" or similar. So it's lock free.

It probably is dead-lock free too. Though that depends on how the code is used. After all, you don't have a "wait" method. Just an "Empty" and "Full" property. And the calling code needs to wait on those.



Since you say "lock free", I assume there are multiple threads using this?

Is that ONE, and no more than just ONE thread for writing?

And
Is that ONE, and no more than just ONE thread for reading?





I had a look at the sources...
Assuming it is ONE/ONE thread...

Even then, it's not save.... 


Code: Pascal  [Select][+][-]
  1. function TRingBuffer.GetEmpty: boolean; inline;
  2. begin
  3.   Result := FReadIndex = FWriteIndex;
  4. end;
GetEmpty may be ok, if you don't care that it can falsely report "empty", after all that is no different to reporting empty, but something being pushed, while the caller still is looking at the boolean result of this. (and the same for falsely reporting "not empty")

Code: Pascal  [Select][+][-]
  1. function TRingBuffer.GetCapacity: PtrUInt; inline;
  2. begin
  3.   if FReadIndex > FWriteIndex then
  4.     Result := FWriteIndex + (PtrUInt.MaxValue - FReadIndex)
  5.   else
  6.     Result := FWriteIndex - FReadIndex;
  7. end;
That can definitely go wrong.

- If you ore the ONLY reader, then between the "if greater" and the "Result :=" the other thread can have changed FWriteIndex.
- And if you are the ONLY writer, then it's vice versa.
- And if you have multiple readers or writers ....

If you know (reader or writer) that one value is save => get a copy of the other value into a local var => so it will not change.
That however means, you must know if GetCapacity is called by the Reader or Writer.

Well actually, get a copy of both, and it works never mind if you are reader or writer (so long as there is only one of each)
Code: Pascal  [Select][+][-]
  1. function TRingBuffer.GetCapacity: PtrUInt; inline;
  2. var r, w: PtrUInt;
  3. begin
  4.   r := FReadIndex;
  5.   w := FWriteIndex;
  6.   {$PUSH}{$R-}{$Q-}
  7.   result := (w - r) and and (FMemorySize - 1); // it seems from MaskIndex, that memsize is a 2^n value.
  8.    {$POP}
  9. end;

Example: MemSize = 1024; ; WriteIndex = 1020; ReadIndex = 1021
  w - r = -1 // -1 in unsigned $ffffffffff // $ffffffffff and 1023 = 1023
And indeed 1023 bytes are in use.



Of course, at the time of the "Result :=" the original may have changed... But it could change at any time after GetCapacity, while you still act on the result...

However:
- If the writer get GetCapacity, the writer only cares that the value will not get less. And *the* (or even any) reader will only increase the Capacity.
- If a reader calls GetCapacity, it only cares that it is less than the total available maximum (i.e. there is data to be read). And the writer can only reduce the value.

So that is ok in both cases.

(Not sure if GetCapacity needs a ReadBarrier)



I haven't looked very deep, and also it can't be told without knowing how this is used by calling code....

But if this is running on any modern Intel/Amd CPU...
They do some unexpected stuff.

They may access variable from memory in a different order, than your code specifies. (Even if FPC kept the order in the generated asm).
They will honour dependencies (like "if (foo <> nil) and (foo.xxx) then". 
But with your indexes, they could access them in unexpected order.

It is a while since I looked at this.... I may have it wrong again (it is mind boggling). But IIRC.

Code: Pascal  [Select][+][-]
  1. procedure TRingBuffer.WriteByte(const AValue: byte);
  2. begin
  3.   pbyte(FMemoryData)[MaskIndex(FWriteIndex)] := AValue;
  4.   WriteBarrier; // Make sure the above byte is in memory, before you increment the index
  5. {$PUSH}
  6. {$Q-}
  7.   Inc(FWriteIndex);
  8. {$POP}
  9. end;
  10.  

Code: Pascal  [Select][+][-]
  1. function TRingBuffer.ReadByte: byte;
  2. begin
  3.   Result := pbyte(FMemoryData)[MaskIndex(FReadIndex)];
  4.   ReadBarrier; // make sure you actually read this, before the other thread gets the increased ReadIndex, and thinks it can overwrite the data.
  5. {$PUSH}
  6. {$Q-}
  7.   Inc(FReadIndex);
  8. {$POP}
  9. end;

And more stuff like this.

Using "Interlocked...." commands (if they translate directly into asm, like they do on intel/amd), is really a lot simpler.
But you may have one or two extra barriers, and in an extreme case blocking you cpu from optimizing by computing ahead, and loosing one or two cpu cycles.... 




If you have more than one thread either reading or writing (on the same buffer), then there is plenty more...





Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 8095
  • Debugger - SynEdit - and more
    • wiki
Re: Heap memory release error message
« Reply #5 on: July 03, 2022, 08:38:10 pm »

Dimitrios Chr. Ioannidis

  • Full Member
  • ***
  • Posts: 167
    • Nephelae
Re: Heap memory release error message
« Reply #6 on: July 03, 2022, 09:43:46 pm »
Hi,

  first, thank you very much for your time. I much appreciate, looking the code and writing a lengthy response !!!

  Let me explain my use case, I need to buffer continuous incoming USB packets ( currently USB 1.1 HID packets, 8 bytes in size, received every 10ms ), for later processing.

Since you say "lock free", I assume there are multiple threads using this?

Is that ONE, and no more than just ONE thread for writing?

And
Is that ONE, and no more than just ONE thread for reading?

  Yes, one buffer reader thread, and one buffer writer thread, in producer ( the buffer writer ) / consumer ( the buffer reader ) setup. 

  After some searching on the internet, I decided that I need a buffer with two indices ( a.k.a free running indices ), which their value change independently from the producer/consumer threads via the ReadByte / WriteByte methods. As I need to maximize the speed as much as possible, the buffer needed to be a ring buffer,  thus the use of PtrUInt and Overflow handling.

Code: Pascal  [Select][+][-]
  1.     function TRingBuffer.GetCapacity: PtrUInt; inline;
  2.     var r, w: PtrUInt;
  3.     begin
  4.       r := FReadIndex;
  5.       w := FWriteIndex;
  6.       {$PUSH}{$R-}{$Q-}
  7.       result := (w - r) and and (FMemorySize - 1); // it seems from MaskIndex, that memsize is a 2^n value.
  8.        {$POP}
  9.     end;
  10.  
Example: MemSize = 1024; ; WriteIndex = 1020; ReadIndex = 1021
  w - r = -1 // -1 in unsigned $ffffffffff // $ffffffffff and 1023 = 1023
And indeed 1023 bytes are in use.

  You're correct . The size must be a power of two ( 2^n ).
  Well, I thought that this method looked suspicious ....  ( my math was wrong :( )

  Thanks ! I'll correct this in the code.

  One question, the double "and" is a typo ?

Code: Pascal  [Select][+][-]
  1.  result := (w - r) and and (FMemorySize - 1);

(Not sure if GetCapacity needs a ReadBarrier)

  Never used them . I'm not familiar with memory barriers . Seems that I need to do some reading, Oh well ....

  What can I say except, thank you very much for the valuable knowledge and for the time to read the code and to wrote that lengthy response, explaining all this.

  regards,
 
« Last Edit: July 03, 2022, 09:53:50 pm by Dimitrios Chr. Ioannidis »
Dimitrios Chr. Ioannidis
http://www.nephelae.eu

Dimitrios Chr. Ioannidis

  • Full Member
  • ***
  • Posts: 167
    • Nephelae
Re: Heap memory release error message
« Reply #7 on: July 03, 2022, 09:58:15 pm »
Hi,

Using "Interlocked...." commands (if they translate directly into asm, like they do on intel/amd), is really a lot simpler.

I'm going to use this code in AVR mcu also, thats why I tried to avoid to use the Interlockedxxxxxx "family". Maybe I'm wrong, need to think about it .

regards,
Dimitrios Chr. Ioannidis
http://www.nephelae.eu

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 8095
  • Debugger - SynEdit - and more
    • wiki
Re: Heap memory release error message
« Reply #8 on: July 03, 2022, 10:14:28 pm »
Ok, then you need to read up yourself.

I don't know how avr works with regards to membarriers. And which ones you need.
I also don't know if they are a single statement, or implemented via other resources.

I don't even know if the problem is even relevant to avr.


PascalDragon

  • Hero Member
  • *****
  • Posts: 4282
  • Compiler Developer
Re: Heap memory release error message
« Reply #9 on: July 04, 2022, 01:45:08 pm »
Using "Interlocked...." commands (if they translate directly into asm, like they do on intel/amd), is really a lot simpler.

I'm going to use this code in AVR mcu also, thats why I tried to avoid to use the Interlockedxxxxxx "family". Maybe I'm wrong, need to think about it .

The Interlocked*-functions on AVR disable and enable interrupts for their operation. Otherwise they don't do any locking and thus might be too much overhead so you could use helper functions that call either the Interlocked*-functions or normal arithmetic functions depending on the platform.

 

TinyPortal © 2005-2018