Segfaults after accumulating many tracks

[vcaputo]

I'm working on something using Rocket for sequencing that tends to create a lot of tracks during the creative process.

Eventually RocketEditor crashes with a SIGSEGV because of this, despite my system having plenty of memory and a willingness to put up with the cluttered tracks.

Cursory investigation revealed this as the issue:

The LinearAllocator can't grow, and doesn't do a graceful ENOMEM equivalent:
https://github.com/emoon/rocket/blob/master/emgui/src/memory/LinearAllocator.c#L24

The LinearAllocator is created from a relatively small fixed-size heap allocation:
https://github.com/emoon/rocket/blob/master/emgui/src/Emgui.c#L344

Eventually I'd like to get the Rocket protocol to support telling the editor when tracks have been removed, that would reduce the number of tracks my use case accumulates during the iterative creative process. But really, even with stale tracks getting actively removed, I'll probably still hit this bug because I'm exposing more variables as tracks and more scenes in the productions which all have unique track groupings.

Anyways, what's the preferred fix upstream here? Should the LinearAllocator be able to realloc itself gracefully when its exhausted? Or just crank up the static size of the malloc and call it Good Enough?

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007f3e0b417635 in ?? () from /usr/lib/libc.so.6
[Current thread is 1 (Thread 0x7f3e0a29ef00 (LWP 187150))]
(gdb) bt full
#0  0x00007f3e0b417635 in  () at /usr/lib/libc.so.6
#1  0x00005629f6e4688e in LinearAllocator_allocAlignedZero ()
#2  0x00005629f6e2fcfd in fillGrad ()
#3  0x00005629f6e2feee in Emgui_fillGrad ()
#4  0x00005629f6e2fe64 in Emgui_fill ()
#5  0x00005629f6e2385f in renderInterpolation ()
#6  0x00005629f6e241d4 in renderChannel ()
#7  0x00005629f6e245dd in renderGroup ()
#8  0x00005629f6e24c51 in TrackView_render ()
#9  0x00005629f6e1b606 in internalUpdate ()
#10 0x00005629f6e1b643 in Editor_update ()
#11 0x00005629f6e2730f in run ()
#12 0x00005629f6e276b3 in main ()

[emoon]

Hey,

Thanks for the report!

I think the prefer fix is is to allow the LinearAllocator to grow. What could be done is just to bump the size and then add the ability to grow. I think this will be pretty rate to happen so it should be fine really (as the realloced LinearAllocator will always be alive the resizing may just happen a very few times)

So to outline it:

In LinearAllocator_allocAligned we should check if there is enough space left. If there isn't we should just realloc the buffer and bump the size times two and store the new size. I think this should be just a few lines of code to implement.

Would you be willing to do a PR with the fix? (I'm currently in the middle of dealing with a flooded basement so I don't have a setup for it right now :( )

[vcaputo]

I'm a bit preoccupied at the moment, but wanted to at least document the issue upstream before completely forgetting about it, since I already bumped the malloc() here to stop the bleeding.

I still want to get the multiclient changes merged upstream so maybe when I'm back on that side of things if this isn't already fixed I'll do that too...

Good luck with the flooding! I've helped family with that in the past, sucks.

[emoon]

Thanks!

[vcaputo]

So to outline it:

In LinearAllocator_allocAligned we should check if there is enough space left. If there isn't we should just realloc the buffer and bump the size times two and store the new size. I think this should be just a few lines of code to implement.

A problem with that simple approach is it would invalidate all the outstanding pointers already handed out via LinearAllocator_allocZero().

And since the existing API is for the caller to provide the space to LinearAllocator_create(), it's awkward at best to make LinearAllocator enlarge/replace that space. The scratchpad use case isn't even passing in a heap-allocated pointer...

It could all be changed to add a new chunk of space when exhausted, chaining to the previous for cleanup handling... but it'll require LinearAllocator_create() owning the memory and not taking it as an input... or maybe if you pass NULL as the space to indicate it's growable, and LinearAllocator allocates it for you, the scratchpad would then be non-growable.

The only sane thing to do without changing the existing dead-simple API is simply enlarge the malloc().

[emoon]

Yeah, that makes sense. Lets just bump the the malloc.