From 3c4d8f65896253a82e19adcbe2808a863a99f74f Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 15 Mar 2018 12:53:56 +0100
Subject: Properly protect player struct with mutex

The volatile keyword neither guarantees atomic access nor memory
visibility[1]. Although this is usually not a problem on x86, it is
incorrect to rely on this. Use mutex locks to protect all shared player
variables and enforce memory visibility.

[1] https://wiki.sei.cmu.edu/confluence/display/c/CON02-C.+Do+not+use+volatile+as+a+synchronization+primitive
---
 src/player.h | 54 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

(limited to 'src/player.h')

diff --git a/src/player.h b/src/player.h
index 0213fde..30107f1 100644
--- a/src/player.h
+++ b/src/player.h
@@ -1,5 +1,5 @@
 /*
-Copyright (c) 2008-2014
+Copyright (c) 2008-2018
 	Lars-Dominik Braun <lars@6xq.net>
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -39,23 +39,31 @@ THE SOFTWARE.
 
 #include "settings.h"
 
+typedef enum {
+	/* not running */
+	PLAYER_DEAD = 0,
+	/* running, but not ready to play music yet */
+	PLAYER_WAITING,
+	/* currently playing a song */
+	PLAYER_PLAYING,
+	/* finished playing a song */
+	PLAYER_FINISHED,
+} BarPlayerMode;
+
 typedef struct {
-	/* protected by pauseMutex */
-	volatile bool doQuit;
-	volatile bool doPause;
-	pthread_mutex_t pauseMutex;
-	pthread_cond_t pauseCond;
-
-	enum {
-		/* not running */
-		PLAYER_DEAD = 0,
-		/* running, but not ready to play music yet */
-		PLAYER_WAITING,
-		/* currently playing a song */
-		PLAYER_PLAYING,
-		/* finished playing a song */
-		PLAYER_FINISHED,
-	} mode;
+	/* public attributes protected by mutex */
+	pthread_mutex_t lock;
+	pthread_cond_t cond; /* broadcast changes to doPause */
+
+	bool doQuit, doPause;
+
+	/* measured in seconds */
+	unsigned int songDuration;
+	unsigned int songPlayed;
+
+	BarPlayerMode mode;
+
+	/* private attributes _not_ protected by mutex */
 
 	/* libav */
 	AVFilterContext *fvolume;
@@ -70,20 +78,18 @@ typedef struct {
 
 	ao_device *aoDev;
 
-	/* settings */
+	/* settings (must be set before starting the thread) */
 	double gain;
 	char *url;
 	const BarSettings_t *settings;
-
-	/* measured in seconds */
-	volatile unsigned int songDuration;
-	volatile unsigned int songPlayed;
 } player_t;
 
 enum {PLAYER_RET_OK = 0, PLAYER_RET_HARDFAIL = 1, PLAYER_RET_SOFTFAIL = 2};
 
 void *BarPlayerThread (void *data);
 void BarPlayerSetVolume (player_t * const player);
-void BarPlayerInit ();
-void BarPlayerDestroy ();
+void BarPlayerInit (player_t * const p, const BarSettings_t * const settings);
+void BarPlayerReset (player_t * const p);
+void BarPlayerDestroy (player_t * const p);
+BarPlayerMode BarPlayerGetMode (player_t * const player);
 
-- 
cgit v1.2.3