Issue with NoteDeleteResponse when submitting requests from FreeRTOS thread

Firmware
1

Hi -

I’m working on an app communicating with a Notecard, using FreeRTOS (on an STM Nucleo-U083RC). When I submit requests to the Notecard from a FreeRTOS thread, and then send the response to the terminal, everything seems to work OK. But I’m concerned about memory leaks, so I have tried to use NoteDeleteResponse after I’ve printed out the response. If I do that, the thread is permanently blocked. I can, however, use JFree on the response and the thread is not blocked, but I don’t think JFree is sufficient (or else why is there NoteDeleteResponse?).

Here’s a sample:

    J *req = NoteNewRequest("hub.get");
    J* rsp = NoteRequestResponseWithRetry(req, 5);
    char* pp = JPrintUnformatted(rsp);
    printf("\n\r** RSP %s **\n\r", pp);
    JFree(pp);
    NoteDeleteResponse(rsp);

As I say, if I comment out the NoteDeleteResponse (or replace with JFree) the thread runs just fine (but memory leaks?), but with the NoteDeleteResponse in place the thread is blocked at that method call.

Since I’m using FreeRTOS, I’ve added wrappers around the thread-safe malloc and free equivalents:

void *tsmalloc(size_t size)
{
    void *ptr = NULL;
    if (size > 0)
    {
        ptr = pvPortMalloc(size);
    }
    return ptr;
}

void tsfree(void *ptr)
{
    if (ptr)
    {
        vPortFree(ptr);
    }
}

… and passed references to NoteSetFn:

  NoteSetFn(tsmalloc, tsfree, delay, millis);

Any ideas what I’m doing wrong here, or what I can try to prevent the thread from being blocked while avoiding memory leaks?

2

Hi @darkerenergy

That’s interesting and unexpected behavior.

Can you create the smallest possible repro of this behavior?

I’d love to flash it on a device and step through it.

Cheers,
Zak

3

Hi Zak -
Thanks for responding! I’m uploading the smallest sample I think I can get away with, that reproduces the behavior. It’s a minor mod of STM’s FreeRTOS_Semaphore_lowPower example, that I’ve used before when working on something with @Youssif. It just launches a thread which blocks waiting for a semaphore. Tapping the blue USER button on the Nucleo releases the semaphore, at which point the thread issues a hub.get request to the Notecard, and then prints the response to the console. This works fine, until I try to do a NoteDeleteResponse (currently commented out). With this line present, the code never reaches the following printf statement.
Let me know if you need more info, or for me to try something…
Cheers,
Chris
Hmmm… How do I upload the code to you Zak? The forum doesn’t want to let me upload a zip file…

4

Perhaps you could create a “throw away” repository on GitHub, and I could download from there?

If it needs to be private, then I’m zfields on GitHub.

5

Hey Zak -
Great idea! :wink: Here it is (public repo)…

Cheers,
Chris

6

Thanks for sharing!

The Application folder is pretty sparse (i.e. I couldn’t find a file with the main loop). Is there anything missing by chance?

Also, I see you provided the binary with the Release folder, but it would be easier to debug if I had all the source files and I could try it on different ST hardware.

7

I’m so sorry @zfields, that will teach me to think something is done, without checking properly. I’ve no idea why my full local repo hasn’t transferred to GitHub. It seems quite random what has actually been sent. But now I can’t find a way to convince Git to send the rest. I do use GitHub a lot, but only from Visual Studio / GitKraken - this is the first time I’ve tried using the STM32CubeIDE/Eclipse integration with Git. Sigh…
I’m not sure how best to proceed. As I mentioned, @Youssif probably still has the sample he and I worked on, which is virtually identical to the one I’m trying to share with you except it doesn’t do the NodeDeleteResponse. Maybe you could grab that from him and we’ll try to synchronize the two? Meanwhile, I’ll look into finding another way to get the repo to you.
Again, apologies for wasting your time…

8

Wow, that was a big mistake. It seems like the Git system has done something nasty. The project in STM32CubeIDE now looks like the remote repo - most of the original files are missing, and I don’t know how to get them back! I’m glad I didn’t try to send you a version of a live project… :worried:

9

Whoa! Thank goodness it was just throw-away code!

I’ll get in touch with @Youssif and we will try to sort this out.

Cheers,
Zak

10

Hey Zak -

Before I tried using the Git integration, I’d made a zip file of the entire project which I’d thought I might be able to upload somewhere here on the forum. So … I sent it to WeTransfer instead, and you should be able to download it using the following URL (good for 7 days):

Cheers,

Chris

11

Hey Chris - we had a look at your code, and there are two main issues leading to this behaviour:-

1. Mixed use of tsmalloc/tsfree and malloc/free
You are correctly passing tsmalloc and tsfree to NoteSetFn(), as they use FreeRTOS’s pvPortMalloc and vPortFree. However, in noteI2CTransmit() and noteI2CReceive(), you are using malloc() and free(). This mixed use is causing an issue as NoteDeleteResponse() eventually calls vPortFree() on memory that is allocated with malloc(). To fix this, you need to use tsmalloc() and tsfree() throughout your code, including in noteI2CTransmit() and noteI2CReceive().

2. Misuse of JStringGet() and JFree()
In app_freertos.c in MainThread_Entry(), JGetString() is being used to get the request string as seen below:-

J *req = NoteNewRequest("hub.get");
char* qq = JGetString(req, "req");
printf("\n\r** REQA %s **\n\r", qq);
JFree(qq);

printf("\n\r** SEND REQUESTA WITH 5 SECOND TIMEOUT **\n\r");
J* rsp = NoteRequestResponseWithRetry(req, 5);
printf("\n\r** RESPONSE RETURNED A **\n\r");

However, JGetString() returns a pointer to the string inside that JSON object, not a copy. Calling JFree(qq) afterwards therefore frees the string inside req, which results in an invalid req being passed to NoteRequestResponseWithRetry(). To fix this, avoid calling JFree on the req object or even JStringGet() in the first place since you already defining the request yourself.

Fixing the above two points should resolve the problem. I was able to reproduce your original behaviour using a Nucleo-U083RC, and after making these corrections, the code worked as expected.

Additional Recommendations
I also want to make these recommendations for future development:-

  • Consider enabling additional log debugging via the NoteSetFnDebugOutput() to print requests and responses. This is especially useful during early stages of development for debugging. Below is an example code snippet of how this would be done:
    size_t noteLogPrint(const char *message_) { 
      if (message_) {
          printf("%s", message_);
          return 1;
      }
      return 0;
    }

    int main() {

      // Other user code here
      NoteSetFnDebugOutput(noteLogPrint);
      // Other user code here
    }

  • This may not be an issue now but if you do more threading work in the future, you should implement the note-c mutex wrappers via NoteSetFnMutex(). This is to ensure safe access to note-c from multiple FreeRTOS tasks. You can see how this is done in the example at the link below:-

    App accelerators: 09 valve monitor

  • Finally, please try to use the latest note-c version. Keeping note-c updated ensures you have the latest fixes and improvements. The latest version can be found here.

Let us know if this resolves your issues or if you need anything else.

Thanks
Youssif

2 Likes
12

Hi @Youssif @zfields -
Thanks for giving me a great start to my week! I really appreciate your assistance with my inability to “see the wood for the trees”. Many thanks for not judging!
I have implemented your correction by fixing the use of malloc() and free() in the Note hooks noteI2CTransmit() and noteI2Receive() [and done a thorough check elsewhere!]. I admit I had rushed to get my MCU talking to my Notecard, and in my hurry I had copy-pasted boilerplate implementations of the hooks - and then stopped thinking about them. Fixing this mixed use of the RTOS memory management and standard memory management removed the problem I was seeing with NoteDeleteResponse() and I have marked your post as the solution. It is (mildly) interesting that we hadn’t previously seen any problem with our (real) app crashing - only when I had been doing some tidying up and added the NoteDeleteResponse().
While I’m thankful for your second point, that’s actually a bit of a red herring. The JFree() had been introduced when I was trying to rapidly construct a “minimum reproducible” to send to @zfields. Nevertheless, it is a very valid point in general, and we need to be less cavalier with our allocation/deallocation.
I’m especially grateful for your additional recommendations. I have somehow managed to get this far without ever coming across NoteSetFnDebugOutput() and we will be adding it to the current build of our app immediately (while we’re still on the steep part of the development curve). I’ll also be taking care of NoteSetFnMutex() as you suggested.
And finally, yep, I will improve our code hygiene and get on the latest version of note-c!
Cheers,
Chris

2 Likes