Possible race condition in att_mtu_throughput example?

[Andres Duarte]

I´m trying to build an application similar to the peripheral side of the app_mtu_throughput example. I need to send a large buffer via notifications to the server.

I'm using an nRF52832 with an S132 and SDK 15.2.0

As I understand it, the att_mtu_throughput example kickstarts the notification process by calling char_notification_send, where it fills the tx buffer of the soft device by calling sd_ble_gatts_hvx until it returns NRF_ERROR_RESOURCES. After that, it sets p_ctx->busy to true and exits the loop. The next time a packet (or several) is sent, a BLE_GATTS_EVT_HVN_TX_COMPLETE event is reported, and if p_ctx->busy was true it means the loop has stopped, so it has to call char_notification_send to fill the buffer up again.

What if all the packets are sent between the call to sd_ble_gatts_hvx that returned NRF_ERROR_RESOURCES and setting p_ctx->busy to true? Then no BLE_GATTS_EVT_HVN_TX_COMPLETE event will observe p_ctx->busy to be true, and the loop would never be restarted. See the example code below.

static void on_tx_complete(nrf_ble_amts_t * p_ctx)
{
    if (p_ctx->busy)
    {
        p_ctx->busy = false;
        char_notification_send(p_ctx);
    }
}

static void char_notification_send(nrf_ble_amts_t * p_ctx)
{
    // (...)
    
    uint32_t err_code = NRF_SUCCESS;
    while (err_code == NRF_SUCCESS)
    {
        // (...)

        err_code = sd_ble_gatts_hvx(p_ctx->conn_handle, &hvx_param); /* FROM HERE */

        if (err_code == NRF_ERROR_RESOURCES)
        {
            // Wait for BLE_GATTS_EVT_HVN_TX_COMPLETE.
            /* TO HERE */ p_ctx->busy = true;
            break;
        }
        
        // (...)
    }
}

If the buffer is emptied between the call to sd_ble_gatts_hvx and p_ctx->busy = true then the loop never restarts. Note that the code is initially run from the main loop, so the Soft Device can interrupt the routine whenever it needs to.

I've never seen the example fail, but I'm worried about a similar implementation failing in the field. How could I make sure that the buffer is not emptied on that critical section? Are there timing restrictions that ensure that that can never happen that I'm not aware of?

Thank you!

[Kenneth]

Hi,

Not seen any reports of it, so I can only assume the risk is very low. Maybe you can add yet another flag that is set in char_notification_send() and cleared in on_tx_complete() that can override the busy if set?

Best regards,
Kenneth

[Andres Duarte]

Thanks for your answer Kenneth.

I admit it looks like a very low-probability risk.

I'm afraid I don't understand your suggestion. Where on the functions do you suggest setting and clearing the new flag?

[Kenneth]

Not sure if my suggestion was a good idea in any case. Maybe you can add a slow running timer (e.g. 100ms) in the background that will call char_notification_send() if p_ctx->busy is true? That should potentially recover the situation if it occurs?

[Andres Duarte]

That could work. I think it means that char_notification_send() would need to be reentrant, or the function would need to be covered by a guard so it does not interrupt itself. But it would restart the cycle.

I am thinking this would solve itself if all calls to char_notification_send() were made from the same interrupt priority, so it cannot interrupt itself. I undertand that the BLE event handlers run under interrupt priority 4. Is my understanding correct? I also read I cannot use that interrupt priority for my application interrupts. I there a way to make the BLE event handlers and an application interrupt run under the same priority?

Thanks for your help Kenneth.

[Kenneth]

Andres Duarte said:

I there a way to make the BLE event handlers and an application interrupt run under the same priority?

Don't think so, but you may use the scheduler (see ble hid examples), then all handling is done in the main context().

[Andres Duarte]

Interesting, I was not aware that module existed. I just read about it and it seems like it would greatly simplify the design of my application, as well as fix my initial concern. I may just use that.

Thank you for your suggestions.