Skip to content

Commit db804a2

Browse files
josesimoesjosesimoes
authored
Fix memory leak with serial port WriteString (#2835)
1 parent d33c0ab commit db804a2

File tree

7 files changed

+96
-20
lines changed

7 files changed

+96
-20
lines changed

‎azure-pipelines.yml‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -722,13 +722,13 @@ jobs:
722722

723723
strategy:
724724
matrix:
725-
ST_B_L475E_IOT01A:
726-
TargetBoard: ST_B_L475E_IOT01A
727-
TargetSeries: 'stm32l4xx'
728-
BuildOptions:
729-
NeedsDFU: true
730-
NeedsSRECORD: false
731-
CMakePreset: ST_B_L475E_IOT01A
725+
# ST_B_L475E_IOT01A:
726+
# TargetBoard: ST_B_L475E_IOT01A
727+
# TargetSeries: 'stm32l4xx'
728+
# BuildOptions:
729+
# NeedsDFU: true
730+
# NeedsSRECORD: false
731+
# CMakePreset: ST_B_L475E_IOT01A
732732

733733
# ORGPAL_PALTHREE:
734734
# TargetBoard: ORGPAL_PALTHREE

‎targets/AzureRTOS/ST/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialPort.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,6 +1242,9 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
12421242
// push onto the eval stack how many bytes are being pushed to the UART
12431243
stack.PushValueI4(bufferLength);
12441244

1245+
// push onto the eval stack if the buffer was allocated
1246+
stack.PushValueI4(isNewAllocation ? 1 : 0);
1247+
12451248
// flush DMA buffer to ensure cache coherency
12461249
// (only required for Cortex-M7)
12471250
cacheBufferFlush(buffer, bufferLength);
@@ -1274,6 +1277,10 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
12741277
if (eventResult)
12751278
{
12761279
// event occurred
1280+
1281+
// pop "isNewAllocation" from the eval stack
1282+
isNewAllocation = stack.m_evalStack[2].NumericByRef().s4 == 1 ? true : false;
1283+
12771284
// get from the eval stack how many bytes were buffered to Tx
12781285
length = stack.m_evalStack[1].NumericByRef().s4;
12791286

@@ -1289,15 +1296,18 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
12891296
// pop "length" heap block from stack
12901297
stack.PopValue();
12911298

1299+
// pop "isNewAllocation" heap block from stack
1300+
stack.PopValue();
1301+
12921302
// pop "hbTimeout" heap block from stack
12931303
stack.PopValue();
12941304

12951305
stack.SetResult_U4(length);
12961306

12971307
// free memory, if it was allocated
1298-
if (isNewAllocation && buffer)
1308+
if (isNewAllocation)
12991309
{
1300-
platform_free(buffer);
1310+
platform_free(palUart->TxBuffer);
13011311
}
13021312

13031313
// null pointers and vars

‎targets/AzureRTOS/SiliconLabs/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialPort.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -939,6 +939,9 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
939939
// push onto the eval stack how many bytes are being pushed to the UART
940940
stack.PushValueI4(bufferLength);
941941

942+
// push onto the eval stack if the buffer was allocated
943+
stack.PushValueI4(isNewAllocation ? 1 : 0);
944+
942945
// store pointer
943946
palUart->TxBuffer = (uint8_t *)buffer;
944947

@@ -968,6 +971,10 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
968971
if (eventResult)
969972
{
970973
// event occurred
974+
975+
// pop "isNewAllocation" from the eval stack
976+
isNewAllocation = stack.m_evalStack[2].NumericByRef().s4 == 1 ? true : false;
977+
971978
// get from the eval stack how many bytes were buffered to Tx
972979
length = stack.m_evalStack[1].NumericByRef().s4;
973980

@@ -983,15 +990,18 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
983990
// pop "length" heap block from stack
984991
stack.PopValue();
985992

993+
// pop "isNewAllocation" heap block from stack
994+
stack.PopValue();
995+
986996
// pop "hbTimeout" heap block from stack
987997
stack.PopValue();
988998

989999
stack.SetResult_U4(length);
9901000

9911001
// free memory, if it was allocated
992-
if (isNewAllocation && buffer)
1002+
if (isNewAllocation)
9931003
{
994-
platform_free(buffer);
1004+
platform_free(palUart->TxBuffer);
9951005
}
9961006

9971007
// null pointers and vars

‎targets/ChibiOS/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialPort.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,6 +1244,9 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
12441244
// push onto the eval stack how many bytes are being pushed to the UART
12451245
stack.PushValueI4(bufferLength);
12461246

1247+
// push onto the eval stack if the buffer was allocated
1248+
stack.PushValueI4(isNewAllocation ? 1 : 0);
1249+
12471250
// flush DMA buffer to ensure cache coherency
12481251
// (only required for Cortex-M7)
12491252
cacheBufferFlush(buffer, bufferLength);
@@ -1276,6 +1279,10 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
12761279
if (eventResult)
12771280
{
12781281
// event occurred
1282+
1283+
// pop "isNewAllocation" from the eval stack
1284+
isNewAllocation = stack.m_evalStack[2].NumericByRef().s4 == 1 ? true : false;
1285+
12791286
// get from the eval stack how many bytes were buffered to Tx
12801287
length = stack.m_evalStack[1].NumericByRef().s4;
12811288

@@ -1291,15 +1298,18 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
12911298
// pop "length" heap block from stack
12921299
stack.PopValue();
12931300

1301+
// pop "isNewAllocation" heap block from stack
1302+
stack.PopValue();
1303+
12941304
// pop "hbTimeout" heap block from stack
12951305
stack.PopValue();
12961306

12971307
stack.SetResult_U4(length);
12981308

12991309
// free memory, if it was allocated
1300-
if (isNewAllocation && buffer)
1310+
if (isNewAllocation)
13011311
{
1302-
platform_free(buffer);
1312+
platform_free(palUart->TxBuffer);
13031313
}
13041314

13051315
// null pointers and vars

‎targets/ESP32/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialPort.cpp‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,6 +1204,7 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
12041204

12051205
bool isNewAllocation = false;
12061206
char *buffer = NULL;
1207+
const char *bufferPointer = NULL;
12071208
uint32_t bufferLength;
12081209
int32_t length = 0;
12091210

@@ -1242,15 +1243,20 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
12421243
// Try to send buffer to fifo first.
12431244
// if not all data written then use long running operation to complete.
12441245
palUart->IsLongRunning = false;
1246+
1247+
// store pointer because it will be changed after this call
1248+
bufferPointer = buffer;
1249+
12451250
int txCount = uart_tx_chars(uart_num, (const char *)buffer, bufferLength);
1251+
12461252
if (txCount < (int)bufferLength)
12471253
{
12481254
palUart->IsLongRunning = true;
12491255
if (txCount >= 0)
12501256
{
12511257
// Any written then update ptr / count
12521258
bufferLength -= txCount;
1253-
buffer += txCount;
1259+
bufferPointer += txCount;
12541260
}
12551261
}
12561262

@@ -1262,14 +1268,21 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
12621268
NANOCLR_CHECK_HRESULT(stack.SetupTimeoutFromTicks(hbTimeout, timeoutTicks));
12631269

12641270
// store pointer
1265-
palUart->TxBuffer = (uint8_t *)buffer;
1271+
palUart->TxBuffer = (uint8_t *)bufferPointer;
12661272

12671273
// set TX count
12681274
palUart->TxOngoingCount = bufferLength;
12691275

12701276
// push onto the eval stack how many bytes are being pushed to the UART
12711277
stack.PushValueI4(bufferLength);
12721278

1279+
// push onto the eval stack if the buffer was allocated
1280+
stack.PushValueI4(isNewAllocation ? 1 : 0);
1281+
1282+
// push buffer pointer to eval stack
1283+
// need this to release the buffer later, because the pointer is changed
1284+
stack.PushValueU4((CLR_UINT32)&bufferPointer);
1285+
12731286
// Create a task to handle UART event from ISR
12741287
char task_name[16];
12751288
snprintf(task_name, ARRAYSIZE(task_name), "uart%d_tx", uart_num);
@@ -1300,6 +1313,13 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
13001313
if (eventResult)
13011314
{
13021315
// event occurred
1316+
1317+
// pop "buffer pointer" from the eval stack
1318+
buffer = (char *)stack.m_evalStack[3].NumericByRef().u4;
1319+
1320+
// pop "isNewAllocation" from the eval stack
1321+
isNewAllocation = stack.m_evalStack[2].NumericByRef().s4 == 1 ? true : false;
1322+
13031323
// get from the eval stack how many bytes were buffered to TX
13041324
length = stack.m_evalStack[1].NumericByRef().s4;
13051325

@@ -1320,14 +1340,20 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
13201340
// pop "length" heap block from stack
13211341
stack.PopValue();
13221342

1343+
// pop "isNewAllocation" heap block from stack
1344+
stack.PopValue();
1345+
1346+
// pop "buffer" heap block from stack
1347+
stack.PopValue();
1348+
13231349
// pop "hbTimeout" heap block from stack
13241350
stack.PopValue();
13251351
}
13261352

13271353
stack.SetResult_U4(length);
13281354

13291355
// free memory, if it was allocated
1330-
if (isNewAllocation && buffer)
1356+
if (isNewAllocation && buffer != NULL)
13311357
{
13321358
platform_free(buffer);
13331359
}

‎targets/FreeRTOS/NXP/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialPort.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,9 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
961961
// push onto the eval stack how many bytes are being pushed to the UART
962962
stack.PushValueI4(bufferLength);
963963

964+
// push onto the eval stack if the buffer was allocated
965+
stack.PushValueI4(isNewAllocation ? 1 : 0);
966+
964967
// store pointer
965968
palUart->TxBuffer = (uint8_t *)buffer;
966969

@@ -987,6 +990,10 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
987990
if (eventResult)
988991
{
989992
// event occurred
993+
994+
// pop "isNewAllocation" from the eval stack
995+
isNewAllocation = stack.m_evalStack[2].NumericByRef().s4 == 1 ? true : false;
996+
990997
// get from the eval stack how many bytes were buffered to Tx
991998
length = stack.m_evalStack[1].NumericByRef().s4;
992999

@@ -1007,15 +1014,18 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
10071014
// pop "length" heap block from stack
10081015
stack.PopValue();
10091016

1017+
// pop "isNewAllocation" heap block from stack
1018+
stack.PopValue();
1019+
10101020
// pop "hbTimeout" heap block from stack
10111021
stack.PopValue();
10121022

10131023
stack.SetResult_U4(length);
10141024

10151025
// free memory, if it was allocated
1016-
if (isNewAllocation && buffer)
1026+
if (isNewAllocation)
10171027
{
1018-
platform_free(buffer);
1028+
platform_free(palUart->TxBuffer);
10191029
}
10201030

10211031
// null pointers and vars

‎targets/TI_SimpleLink/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialPort.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -976,6 +976,9 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
976976
// push onto the eval stack how many bytes are being pushed to the UART
977977
stack.PushValueI4(bufferLength);
978978

979+
// push onto the eval stack if the buffer was allocated
980+
stack.PushValueI4(isNewAllocation ? 1 : 0);
981+
979982
// store pointer
980983
palUart->TxBuffer = (uint8_t *)buffer;
981984

@@ -997,6 +1000,10 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
9971000
if (eventResult)
9981001
{
9991002
// event occurred
1003+
1004+
// pop "isNewAllocation" from the eval stack
1005+
isNewAllocation = stack.m_evalStack[2].NumericByRef().s4 == 1 ? true : false;
1006+
10001007
// get from the eval stack how many bytes were buffered to Tx
10011008
length = stack.m_evalStack[1].NumericByRef().s4;
10021009

@@ -1014,15 +1021,18 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeWriteString_
10141021
// pop "length" heap block from stack
10151022
stack.PopValue();
10161023

1024+
// pop "isNewAllocation" heap block from stack
1025+
stack.PopValue();
1026+
10171027
// pop "hbTimeout" heap block from stack
10181028
stack.PopValue();
10191029

10201030
stack.SetResult_U4(length);
10211031

10221032
// free memory, if it was allocated
1023-
if (isNewAllocation && buffer)
1033+
if (isNewAllocation)
10241034
{
1025-
platform_free(buffer);
1035+
platform_free(palUart->TxBuffer);
10261036
}
10271037

10281038
// null pointers and vars

0 commit comments

Comments
 (0)