Crash on file::read in C-API

SDK Bug Reports
1

It seems like the playdate APIs file read function either thrashes the memory or is leaking memory triggering a crash later on. This only happens on the physical device not in the simulator.

I've tested this out on Linux in Playdate version "1.11" and "2.0.0.beta.2" and both seem to show the same behaviour.

How to reproduce:

  1. Have a large enough file to load (I've got a BMP file that I'm reading that is 2.1 MB large for my example)
  2. Run the following code somewhere in your game (either in setup or main-loop of the game should be fine)
unsigned char *loadBMPDataFromImgMap(PlaydateAPI *pd, unsigned char* file, int xOffset, int yOffset, int targetWidth, int targetHeight)
{
	char header[54];
	int dataPos;
	int imageSize;
    unsigned char *data = NULL;

    unsigned char *data_2bitsPerPixel = NULL;

    int width;
    int height;
    SDFile* bmpData = pd->file->open(file, kFileRead);

    if(bmpData)
    {
        pd->system->logToConsole("loadBMPDataFromImgMap read %s", file);

        // Read the header
        pd->file->read(bmpData, header, sizeof(char) * 54);
        
	    dataPos    = *(int*)&(header[0x0A]);
	    imageSize  = *(int*)&(header[0x22]);        
	    width     = *(int*)&(header[0x12]);
	    height    = *(int*)&(header[0x16]);

        // Some BMP files are misformatted, guess missing information
	    if (imageSize == 0)
        {
            imageSize = width * height * 3; // 3 : one byte for each Red, Green and Blue component
            pd->system->logToConsole("Image size missing");
        }
	    if (dataPos == 0)
        {
            dataPos = 54; // The BMP header is done that way            
        }

        pd->system->logToConsole("dataPos %d", dataPos);
        
        // Lock memory for the BMP data
        data = pd->system->realloc(NULL, imageSize * sizeof(unsigned char)); 

        pd->system->logToConsole("locked mem for data %p, size : %d", data, imageSize);      

        // Read the data and then close the file
        int dataRead = pd->file->read(bmpData, data, sizeof(unsigned char) * imageSize);
        pd->file->close(bmpData);    

        bmpData = NULL;

        pd->system->logToConsole("read back %d from file %s", dataRead, file);

        pd->system->realloc(data, 0);
        
        pd->system->logToConsole("freed data");
    }    

    return NULL;
}

// Put this inside setup call or in the main game loop
for(int i = 0; i < 100; i++)
{
    loadBMPDataFromImgMap(pd, "images/guard_walk_h.bmp", 0, 0, 0, 0);
}

(xoffset, yoffset, targetWidth, and targetHeight are not used in the function)

After a couple of iterations in the foor loop calling loadBMPDataFromImgMap the physical device crashes. If you remove the following line from the function the crash goes away:
int dataRead = pd->file->read(bmpData, data, sizeof(unsigned char) * imageSize);

It seems like file->read is the culprit. I did try running the same for loop in setup but with much fewer iterations (10 iterations) and I'm not able to see any memory leaks (I've tested a version where loadBMPDataFromImgMap never gets called and the memory foot print seems to be the exact same).

I'm probably able to work around this by simply loading my image map once and just reusing the data from the file - but there seems like there is some issue with the file read function here that mainly shows up when reading large quantities of data (I never saw this issue until I started loading in my larger "image maps" that contains frames of animation). My code just opens/closes the file repetedly as it is building each individual frame but I can refactor it to just re-use the data I've loaded.
This does however seem like a bug in the playdate API and would ofc be good to have this fixed.

Kindly,
/Jimmie

2

I've created an even smaller example of how to reproduce this and uploaded a minimal game that will build and crash on the device (so far I've only tested it on version 1.11 but I'm pretty sure it will crash on latest as well).

The source and a sample BMP image to test it out with is sitting here:

I rewrote the loadbmp function so its does even less, now it looks like this:

unsigned char *loadBMPDataFromImgMap(PlaydateAPI *pd, unsigned char* file)
{
    SDFile* bmpData = pd->file->open(file, kFileRead);

    if(bmpData)
    {
        const int imageSize = 2150454;
        pd->system->logToConsole("loadBMPDataFromImgMap read %s", file);

        unsigned char* data = pd->system->realloc(NULL, imageSize * sizeof(unsigned char));
        int dataRead = pd->file->read(bmpData, data, sizeof(unsigned char) * imageSize);

        pd->system->logToConsole("loadBMPDataFromImgMap read %d", dataRead);
        pd->file->close(bmpData);
        bmpData = NULL;

        pd->system->realloc(data, 0);
    }    

    return NULL;
}

// game initialization
void setupGame(void)
{
	srand(pd->system->getSecondsSinceEpoch(NULL));

    pd->system->logToConsole("Running setupGame");
    
    // This will cause the device to crash
    for(int i = 0; i < 100; i++)
    {
        loadBMPDataFromImgMap(pd, "images/guard_walk_h.bmp");
    }
}

I've built it for the device like this:
cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_TOOLCHAIN_FILE=/home/myUser/Documents/PlaydateSDK-1.11.0/C_API/buildsupport/arm.cmake ..

Cheers,
/Jimmie

3

@dave do you have time to investigate this?

5

I'm looking at it right now. It's hitting the 10 second runloop timeout, but there's a bug where it crashes instead of showing the timeout error. The issue is that the error handler is getting called from a timer interrupt with a higher priority than the systick timer, and the normal filesystem locking code doesn't work here. I can work around this by disabling the FreeRTOS scheduler before calling showerror() but I'm trying to find a less drastic way. Though I guess in this case we've decided the main task is blocked, so waiting for it to let go of the filesystem lock isn't really feasible.. Maybe we just shouldn't log the error to disk if the disk is in use when the timeout fires.

6

Yeah, that works--error screen showing a runloop timeout now appears correctly--and I feel a lot better about skipping the logging than brute forcing into the filesystem. :+1:

7

Hi,

Cool, thanks for testing out and verifying the bug :slight_smile:

Can I work around this by for example in the ”main game loop” loading a few images and then letting that ”main loop” function return 1 and then just resume by keeping track myself of which files I have loaded? I could defently live with that.

Again, thanks for taking the time and having a look at this!

/Jimmie

8

Just quick note if anyone else is reading this and having the same issue - making several calls in the ”main game loop” function to load files does work as a way to get around this problem.

/Jimmie

2 Likes
9

In Lua you can avoid run loop timeouts with coroutine.yield() (though not at load time) which makes things a little easier. In C you're a little closer to the metal so we don't have that option. I haven't looked at this C coroutine library for Playdate but it looks interesting, might help here: [C/C++] Coroutines Library for Playdate

1 Like