vectorgrp / XCPlite

Simple implementation of the ASAM XCP on Ethernet protocol
MIT License
171 stars 93 forks source link

Fix flaky server shutdown mechanism #55

Closed truecarfield closed 2 months ago

truecarfield commented 2 months ago

What?

Why?

RainerZ commented 2 months ago

Hello,

Thanks.

Do you have an idea, what the root cause for your crashes was ?

There seems to be another problem: XcpEthServerShutdown() calls XcpTlShutdown(), which should be XcpEthTlShutdown(). Right ?

I was unsure what the optimal solution - graceful or forceful thread shutdown - should be. Windows strongly recommends graceful thread termination, as better programming style ,to avoid resource leaks in general.

Seems the reinitialization use case has never been stressed. Of course this should work, but what exactly is your use case why you do reinitialization.

RainerZ commented 2 months ago

I did some more testing on the 2 solutions after fixing XcpEthTlShutdown():

'''

ifdef XCP_SERVER_FORCEFULL_TERMINATION

// Forcefull termination
if (gXcpServer.isInit) {
    DBG_PRINT3("Disconnect, cancel threads and shutdown XCP!\n");
    XcpDisconnect();
    cancel_thread(gXcpServer.ReceiveThreadHandle);
    cancel_thread(gXcpServer.TransmitThreadHandle);
    XcpEthTlShutdown();
    gXcpServer.isInit = FALSE;
    socketCleanup();
    XcpReset();
}

else

// Gracefull termination
if (gXcpServer.isInit) {
    XcpDisconnect();
    gXcpServer.ReceiveThreadRunning = FALSE;
    gXcpServer.TransmitThreadRunning = FALSE;
    XcpEthTlShutdown();
    join_thread(gXcpServer.ReceiveThreadHandle);
    join_thread(gXcpServer.TransmitThreadHandle);
    gXcpServer.isInit = FALSE;
    socketCleanup();
    XcpReset();
}

endif

'''

I am not lucky with the forcefull alternative, as it seems to start a race between cancel_thread and shutting down the socket, which leads to graceful termination via recvfrom. Did not check TCP yet.

RainerZ commented 2 months ago

Pushed changes to master v6.3 Added OPTION_SERVER_FORCEFULL_TERMINATION