nusserver example panics on NRF51 (microbit-v1)

[HattoriHanzo031]

When running unmodified nusserver example on microbit-v1 after connecting and disconnecting to the device, following panic occurs:

starting
NUS console enabled, use Ctrl-X to exit
panic: runtime error at 0x00019161: heap alloc in interrupt
[tinygo: panic at /.../bluetooth/gap_nrf51.go:73:2]

Also, if I try to send any message from the client, following panic occurs:

starting
NUS console enabled, use Ctrl-X to exit
panic: runtime error at 0x000193bf: heap alloc in interrupt
[tinygo: panic at /.../bluetooth/gatts_sd.go:113:3]

I am using nRF Toolbox Android app as a client.

The first panic occurs in:

func (a *Advertisement) start() uint32 {
	params := C.ble_gap_adv_params_t{
		...
	}
	return C.sd_ble_gap_adv_start(&params)
}

where params escapes to heap since escape analysis can't look into C.sd_ble_gap_adv_start.
start is called on BLE_GAP_EVT_DISCONNECTED event in handleEvent function, hence the panic.

Similar thing happens in second case, and the reason is that in example WriteEvent of RX Characteristics writes back to TX Characteristics, and this Write calls C.sd_ble_gatts_hvx that will escape the pointer to heap.

I have made a dirty workaround for this issue on my fork which solves both issue by using global params variables, but I don't think this is a good solution.

Looking at the NRF SDK code it is clear that the sd_ble_gatts_hvx call does not need heap allocated argument since in SDK stack variable is used:

        ble_gatts_hvx_params_t hvx_params;
        ...
        hvx_params.handle = p_bps->meas_handles.value_handle;
        hvx_params.type   = BLE_GATT_HVX_INDICATION;
        ...
        err_code = sd_ble_gatts_hvx(p_bps->conn_handle, &hvx_params);

I think the proper solution would be to somehow force the compiler not to escape the params to heap, but I didn't find a way to do this. I tried the trick from src/strings/builder.go noescape function, but it didn't work (or I am doing something wrong)

[HattoriHanzo031]

I have found another workaround to prevent C call to cause heap allocation. I wrote a simple wrapper CGO function that accepts a struct and calls actual function with pointer to that struct:

uint32_t sd_ble_gap_adv_start_noescape(ble_gap_adv_params_t const p_adv_params) {
	return sd_ble_gap_adv_start(&p_adv_params);
}

It is not perfect since it creates unnecessary copy of a struct, but it is much better solution than using a global.
Convincing compiler not to escape pointer to heap would be even better, but I don't know of a way to do this. If anyone knows how, please leave a comment.
There is an accepted proposal golang#56378 to add CGO noescape annotation in Go which would solve this sort of issues.

[aykevl]

@HattoriHanzo031 I think that wrapper function is a nice workaround. It's probably inlined so the big parameter doesn't really matter (especially if you make the wrapper function static).

Can you make a PR out of this?

Also, that CGo noescape proposal looks quite interesting, that's another way this issue could be fixed.

[HattoriHanzo031]

PR #192 created

[aykevl]

I've implemented #cgo noescape here (but unfortunately it'll take a while before it can be used): tinygo-org/tinygo#3887