mirror/bacnet-stack
1
0
Fork 0
mirror of https://github.com/stargieg/bacnet-stack synced 2025-10-19 23:25:23 +08:00
Code Issues Releases Wiki Activity

[r3138] Merged revision(s) 3136 from branches/releases/bacnet-stack-0-8-0:

Browse Source 
Replace Receive_Packet_Flag conditional variable with a semaphore and update the related library functions accordingly.
Analysis of the problem determined that the issue lay in the transfer of APDU packets between the FSM and the APDU packet handler thread.
The mechanism previously used by the FSM to notify the APDU packet handler thread that a packet was available for processing used a pthread conditional variable which packet handler thread was supposed to wait on before being signalled by the FSM.
However the packet handler thread has other tasks to perform and sometimes was not waiting on the conditional variable which it was signalled.
Unlike other synchronisation mechanisms such as semaphores, if the waiting task (the consumer) is not blocked on the conditional variable when the producer signals, then that signal is lost and the consumer is never signalled again, leading to a continual sequence of timeouts on the conditional variable.
This in turn led to the packet handler thread never being notified of a packet waiting to be processed thus causing the interface hang.
The main problem is that a conditional variable is supposed to be used with a mutex to prevent this behaviour occurring, but this mutex was not present (and in fact had been removed from the code, most likely because it was causing other synchronisation issues)
Further inspection revealed that this code was copied from another file but modified to remove the mutex which is an essential part of using a conditional variable for synchronisation. This then prevents the producer task being blocked until the consumer task is waiting on the conditional variable, thus leading to a race condition which is causing the issues seen.
The fix is to replace the conditional variable with a semaphore as this provides the required mechanism in this case.
Thank you Ian Smith at Abelon Systems Ltd. for the patch!

........
This commit is contained in:
Steve Karg 2017-06-25 20:08:36 +02:00 committed by Patrick Grimm
parent 17058b23b3
commit 0f0fb2664d
2 changed files with 7 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ports/linux/dlmstp_linux.c
View File

@ -123,10 +123,9 @@ void dlmstp_cleanup(
close(poSharedData->RS485_Handle);
pthread_cond_destroy(&poSharedData->Received_Frame_Flag);
pthread_cond_destroy(&poSharedData->Receive_Packet_Flag);
sem_destroy(&poSharedData->Receive_Packet_Flag);
pthread_cond_destroy(&poSharedData->Master_Done_Flag);
pthread_mutex_destroy(&poSharedData->Received_Frame_Mutex);
pthread_mutex_destroy(&poSharedData->Receive_Packet_Mutex);
pthread_mutex_destroy(&poSharedData->Master_Done_Mutex);
}
@ -192,8 +191,7 @@ uint16_t dlmstp_receive(
/* see if there is a packet available, and a place
to put the reply (if necessary) and process it */
get_abstime(&abstime, timeout);
rv = pthread_cond_timedwait(&poSharedData->Receive_Packet_Flag,
&poSharedData->Receive_Packet_Mutex, &abstime);
rv = sem_timedwait(&poSharedData->Receive_Packet_Flag, &abstime);
if (rv == 0) {
if (poSharedData->Receive_Packet.ready) {
if (poSharedData->Receive_Packet.pdu_len) {
@ -358,7 +356,7 @@ uint16_t MSTP_Put_Receive(
mstp_port->SourceAddress);
poSharedData->Receive_Packet.pdu_len = mstp_port->DataLength;
poSharedData->Receive_Packet.ready = true;
pthread_cond_signal(&poSharedData->Receive_Packet_Flag);
sem_post(&poSharedData->Receive_Packet_Flag);
}
return pdu_len;
@ -910,19 +908,13 @@ bool dlmstp_init(
/* initialize packet queue */
poSharedData->Receive_Packet.ready = false;
poSharedData->Receive_Packet.pdu_len = 0;
rv = pthread_cond_init(&poSharedData->Receive_Packet_Flag, NULL);
rv = sem_init(&poSharedData->Receive_Packet_Flag, 0, 0);
if (rv != 0) {
fprintf(stderr,
"MS/TP Interface: %s\n cannot allocate PThread Condition.\n",
ifname);
exit(1);
}
rv = pthread_mutex_init(&poSharedData->Receive_Packet_Mutex, NULL);
if (rv != 0) {
fprintf(stderr,
"MS/TP Interface: %s\n cannot allocate PThread Mutex.\n", ifname);
exit(1);
}
struct termios newtio;
printf("RS485: Initializing %s", poSharedData->RS485_Port_Name);

7
ports/linux/dlmstp_linux.h
View File

@ -27,6 +27,7 @@
#include "mstp.h"
/*#include "dlmstp.h" */
/*#include "bits/pthreadtypes.h"*/
#include <semaphore.h>
#include <stdbool.h>
#include <stdint.h>
@ -70,11 +71,9 @@ typedef struct shared_mstp_data {
DLMSTP_PACKET Receive_Packet;
DLMSTP_PACKET Transmit_Packet;
/*
RT_COND Receive_Packet_Flag;
RT_MUTEX Receive_Packet_Mutex;
RT_SEM Receive_Packet_Flag;
*/
pthread_cond_t Receive_Packet_Flag;
pthread_mutex_t Receive_Packet_Mutex;
sem_t Receive_Packet_Flag;
/* mechanism to wait for a frame in state machine */
/*
RT_COND Received_Frame_Flag;