When extending a existing memory block with realloc which may contain pointers into the D GC heap the first implementation that comes to mind is:
void* reallocImpl(void* oldPtr, size_t newSize)
{
void* newPtr = realloc(oldPtr, newSize);
//GC Point 1
GC.addRange(newPtr);
//GC Point 2
GC.removeRange(oldPtr);
return newPtr;
}
This implementation has many issues however.
1) If realloc allocated a new block of memory and freed the old one, the following issue arises. If the GC triggres at either GC Point 1 or GC Point 2 (from a different thread), the GC will read the old memory block which is freed. The c-spec states that you should not read this memory after calling realloc.
2) If realloc did not allocate a new block but extended the existing one, the GC.addRange call will not do anything because the underling Treap container ignores duplicates. The remove range call will then remove the old memory block from the list and as a result the GC will loose any knowdelge about this memory block.
So a better implementation would look like this.
void* reallocImpl(void* oldPtr, size_t newSize)
{
GC.disable();
GC.removeRange(oldPtr);
// GC Point 1
void* newPtr = realloc(oldPtr, newSize);
// GC Point 2
GC.addRange(newPtr);
GC.enable();
return newPtr;
}
This would work if GC.disable would actually guarantee that the GC never runs. But as the GC might still run in out of memory situations it can happen that is triggered in either GC Point 1 or GC Point 2. If this happens the GC will not scan either the old or the new memory block.
I suggest that we add a function to the GC interface that allows safe updating of a memory range
void* updateRange(void* ptr, scope void* delegate() updateFunc);
updateFunc would be called within the GC lock, preventing any of the issues described above.
void* reallocImpl(void* oldPtr, size_t newSize)
{
return GC.updateRange(oldPtr, (){ return realloc(oldPtr, newSize); });
}
Comment #1 by code — 2015-08-19T14:20:25Z
And don't even think about what happens when a collection is triggered while realloc copies the data from the old array to the new array.
> updateFunc would be called within the GC lock, preventing any of the issues described above.
Please let's try to avoid arbitrary code execution with the GC lock held.
> This would work if GC.disable would actually guarantee that the GC never runs.
This is what http://dlang.org/phobos/core_thread.html#.thread_enterCriticalRegion and http://dlang.org/phobos/core_thread.html#.thread_exitCriticalRegion are for, using them should readily solve your problem.
void* reallocImpl(void* p, size_t newSize)
{
thread_enterCriticalRegion();
auto oldp = p;
p = realloc(p, newSize);
if (p !is oldp)
{
GC.removeRange(oldp);
GC.addRange(p, newSize);
}
thread_exitCriticalRegion();
return newPtr;
}
Comment #2 by code — 2015-08-19T14:25:52Z
Maybe there could be a dead-lock in acquiring the GC lock in a critical region. Need to recheck that.
Comment #3 by schveiguy — 2015-08-19T16:34:11Z
(In reply to Martin Nowak from comment #1)
> Please let's try to avoid arbitrary code execution with the GC lock held.
I really don't see another way to do this. Your idea of using enterCriticalRegion isn't viable, because you need to call removeRange and addRange (which take the lock, that might be held while attempting to suspend the threads). Unless I misunderstand how it works.
However, I don't think it needs to be an "advertised" feature. I'm not sure we need to do it via a delegate, all we need is exposure to the lock. This needs to be done only in low-level situations.
> void* reallocImpl(void* p, size_t newSize)
> {
> thread_enterCriticalRegion();
> auto oldp = p;
> p = realloc(p, newSize);
> if (p !is oldp)
> {
> GC.removeRange(oldp);
> GC.addRange(p, newSize);
> }
> thread_exitCriticalRegion();
> return newPtr;
> }
Note, you need to remove and re-add the range in both cases, because the new range is bigger.
Comment #4 by robert.schadek — 2024-12-07T13:35:40Z