diff --git a/components/squeezelite/stream.c b/components/squeezelite/stream.c index 73d097ce..83ae4b07 100644 --- a/components/squeezelite/stream.c +++ b/components/squeezelite/stream.c @@ -46,27 +46,17 @@ struct buffer *streambuf = &buf; #define UNLOCK mutex_unlock(streambuf->mutex) /* -After a lot of hesitation, I've added that "poll mutex" to prevent -socket from being allocated while we are still in poll(). The issue -happens is we have a close quickly followed by an open, we might still -be in the poll() and simple OS fail as they re-allocate the same socket -on which a thread is still waiting. -Ideally, you want to set the lock in the disconnect() but that would mean -very often we'd have to always wait for the end of the poll(), i.e. up to -100ms for nothing most of the time where if it is in the open(), it is -less elegant as closing a socket on which there is a poll() is not good -but it's more efficient as it is very rare that you'd have an open() less -then 100ms after a close() +When LMS sends a close/open sequence very quickly, the stream thread might +still be waiting in the poll() on the closed socket. It is never recommended +to have a thread closing a socket used by another thread but it works, as +opposed to an infinite select(). +In stream_sock() a new socket is created and full OS will allocate a different +one but on RTOS and simple IP stack, the same might be re-used and that causes +an exception as a thread is already waiting on a newly allocated socket +A simple variable that forces stream_sock() to wait until we are out of poll() +is enough and much faster than a mutex */ -#if EMBEDDED -static mutex_type poll_mutex; -#define LOCK_L mutex_lock(poll_mutex) -#define UNLOCK_L mutex_unlock(poll_mutex) -#else -#define LOCK_L -#define UNLOCK_L -#endif - +static bool polling; static sockfd fd; struct streamstate stream; @@ -209,7 +199,7 @@ static void *stream_thread() { } else { - LOCK_L; + polling = true; pollinfo.fd = fd; pollinfo.events = POLLIN; if (stream.state == SEND_HEADERS) { @@ -221,7 +211,7 @@ static void *stream_thread() { if (_poll(ssl, &pollinfo, 100)) { - UNLOCK_L; + polling = false; LOCK; // check socket has not been closed while in poll @@ -374,7 +364,8 @@ static void *stream_thread() { UNLOCK; } else { - UNLOCK_L; + // it is safe to set it unlocked + polling = false; LOG_SDEBUG("poll timeout"); } } @@ -427,9 +418,6 @@ void stream_init(log_level level, unsigned stream_buf_size) { *stream.header = '\0'; fd = -1; -#if EMBEDDED - mutex_create_p(poll_mutex); -#endif #if LINUX || FREEBSD touch_memory(streambuf->buf, streambuf->size); @@ -459,9 +447,6 @@ void stream_close(void) { #endif free(stream.header); buf_destroy(streambuf); -#if EMBEDDED - mutex_destroy(poll_mutex); -#endif } void stream_file(const char *header, size_t header_len, unsigned threshold) { @@ -503,9 +488,13 @@ void stream_file(const char *header, size_t header_len, unsigned threshold) { void stream_sock(u32_t ip, u16_t port, const char *header, size_t header_len, unsigned threshold, bool cont_wait) { struct sockaddr_in addr; - LOCK_L; +#if EMBEDDED + // wait till we are not polling anymore + for (LOCK; running && polling; UNLOCK, usleep(10000), LOCK); + UNLOCK; +#endif + int sock = socket(AF_INET, SOCK_STREAM, 0); - UNLOCK_L; if (sock < 0) { LOG_ERROR("failed to create socket");