diff --git a/src/bacnet/basic/bbmd/h_bbmd.c b/src/bacnet/basic/bbmd/h_bbmd.c index 4230f592f3..1582bf7ade 100644 --- a/src/bacnet/basic/bbmd/h_bbmd.c +++ b/src/bacnet/basic/bbmd/h_bbmd.c @@ -38,6 +38,7 @@ #include /* for memcpy */ #include #include +#include #include "bacnet/bacdcode.h" #include "bacnet/datalink/bip.h" #include "bacnet/datalink/bvlc.h" @@ -100,9 +101,16 @@ typedef enum BBMD_REG_SUCCESS } bbmd_reg_t; -/** Mutex and condition variable for checking if BBMD registration has been successful */ -static pthread_mutex_t mutex; -static pthread_cond_t cond; +/** Mutex and condition variable for checking if BBMD registration has been + * successful. Statically initialized (not pthread_*_init()'d at runtime) + * because the BVLC_RESULT handler below locks/signals them unconditionally + * on any incoming BVLC_RESULT, including one that arrives before + * bvlc_register_with_bbmd() is ever called -- using them before a runtime + * init would be undefined behavior otherwise. Never destroyed, for the + * same reason: they must stay valid for the life of the process, not just + * for the duration of one registration attempt. */ +static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; static bbmd_reg_t bbmd_reg; /** @@ -1160,41 +1168,39 @@ int bvlc_register_with_bbmd(BACNET_IP_ADDRESS *bbmd_addr, uint16_t ttl_seconds) BVLC_Buffer_Len = bvlc_encode_register_foreign_device( &BVLC_Buffer[0], sizeof(BVLC_Buffer), ttl_seconds); - pthread_mutex_init (&mutex, NULL); - pthread_mutex_lock (&mutex); /* Set the initial value of the BBMD registration bool to false */ bbmd_reg = BBMD_REG_UNSET; pthread_mutex_unlock (&mutex); - /* Setup a 30 second condition variable wait */ - pthread_cond_init (&cond, NULL); - time_t timeout_seconds = 3; - struct timeval now; - struct timespec timeout; - int timeout_count = 0; - int retval = bip_send_mpdu(bbmd_addr, &BVLC_Buffer[0], BVLC_Buffer_Len); if (retval == -1) { return retval; } - while (timeout_count < 10) + + /* Nothing else reads the BIP socket at this point in startup -- the + * main receive loop (bip_receive() via datalink_receive()) hasn't + * started yet, so bvlc_handler() can never run to set bbmd_reg unless + * THIS call also pumps the socket itself. A pthread_cond_timedwait() + * here (the original approach) can never be signaled: nothing else in + * this process reads the BBMD's real ACK/NAK off the wire during the + * wait, so it always timed out and reported failure even when the + * BBMD's registration genuinely succeeded. Poll bip_receive() + * directly instead -- it invokes bvlc_handler() internally on any + * packet received, which is what actually sets bbmd_reg. */ + BACNET_ADDRESS bbmd_reply_src = { 0 }; + uint8_t bbmd_reply_mtu[MAX_MPDU] = { 0 }; + time_t deadline = time(NULL) + 30; + while (time(NULL) < deadline) { - gettimeofday (&now, NULL); - timeout.tv_sec = now.tv_sec + timeout_seconds; - timeout.tv_nsec = 0; + bip_receive(&bbmd_reply_src, bbmd_reply_mtu, sizeof(bbmd_reply_mtu), 100); pthread_mutex_lock (&mutex); - pthread_cond_timedwait (&cond, &mutex, &timeout); bool received_response = bbmd_reg != BBMD_REG_UNSET; pthread_mutex_unlock (&mutex); if (received_response) break; - timeout_count++; } - pthread_cond_destroy(&cond); - pthread_mutex_destroy(&mutex); - /* Fail if the BBMD registration was not successful */ if (bbmd_reg != BBMD_REG_SUCCESS) {