Discussion :

[Bktero]

Bonjour,

Dans le cadre d'un développement embarqué, j'ai besoin de contrôler l'accès à un bus SPI. L'objectif est d'empêcher les utilisations concurrentes du bus. Pour cela, on doit respecter l'enchainement :

1. lock
2. utilisation
3. unlock

En fan de Scott Meyers, j'applique l'adage "les API doivent être simple à utiliser correctement et difficile à utiliser incorrectement". Il faut :

• ne pas pouvoir oublier de faire unlock() --> RAII
• ne pas pouvoir travailler sans avoir fait lock() --> renvoyer un objet permettant l'utilisation quand on fait lock()

J'aimerais vous montrer mon code pour que vous me donniez vos avis. Les commentaires décrivent les buts de chaque classe.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
#include <iostream>
#include <mutex>
 
class Mutex {
/*
 * arm-gcc-none-eabi ne fournit pas std::mutex.
* j'ai donc recodé une classe avec la même API
*/
public:
	void lock() {
		std::cout << __PRETTY_FUNCTION__ << std::endl;
		mutex_m.lock();
	}
 
	void unlock() {
		std::cout << __PRETTY_FUNCTION__ << std::endl;
		mutex_m.unlock();
	}
 
private:
	std::mutex mutex_m; // pour remplacer les mutex natives lors d'un test sur PC
};
 
/*
 * La ressource materielle.
 * Le code applicatif n'utilise pas directement cette classe.
 */
class Spi {
public:
	bool send(std::uint8_t byte) const {
		std::cout << __PRETTY_FUNCTION__ << " " << (int) byte << std::endl;
		return true;
	}
};
 
/*
 * Le contrôleur des accès concurrents sur le bus.
 */
class SpiBus {
public:
	/*
	 * Objet renvoyé par SpiBus::lock()
	 */
	class LockedSpiBus : public Spi {
	public:
		/*
		 * RAII: on verrouille mutex à la création
		 */
		LockedSpiBus(Mutex& mutex) :
				mutex_m(mutex) {
			std::cout << __PRETTY_FUNCTION__ << std::endl;
			mutex_m.lock();
		}
 
		/*
		 * RAII: on déverouille à la destruction
		 */
		~LockedSpiBus() {
			std::cout << __PRETTY_FUNCTION__ << std::endl;
			mutex_m.unlock();
		}
 
		/*
		 * Il y plus de fonctionnalités dans LockedSpiBus que dans Spi,
		 * comme la sélection d'adresse sur le bus.
		 */
		void workWithBus() {
			std::cout << __PRETTY_FUNCTION__ << std::endl;
		}
	private:
		Mutex& mutex_m;
	};
 
	/*
	 * Renvoyer un SpiLockBus permet d'éviter de travailler sur le bus avant de l'avoir verrouiller.
	 */
	LockedSpiBus lock() {
		std::cout << __PRETTY_FUNCTION__ << std::endl;
		auto locked = LockedSpiBus(mutex_m);
		return locked;
	}
 
	/*
	 * Peut-être supprimée ?
	 */
	void unlock() {
		mutex_m.unlock();
		std::cout << __PRETTY_FUNCTION__ << std::endl;
	}
 
private:
	Mutex mutex_m;
};
 
/*
 * Exemple d'utilisation.
 */
int main() {
 
	SpiBus bus;
 
	auto locked_bus = bus.lock();
 
	locked_bus.workWithBus();
	locked_bus.send(42);
 
	//bus.unlock(); // optionnel
}

Le sortie console montre que ça se passe bien :

SpiBus::LockedSpiBus SpiBus::lock()
SpiBus::LockedSpiBus::LockedSpiBus(Mutex&)
void Mutex::lock()
void SpiBus::LockedSpiBus::workWithBus()
bool Spi::send(uint8_t) const 42
SpiBus::LockedSpiBus::~LockedSpiBus()
void Mutex::unlock()

Pour ne pas alourdir le code, je n'ai pas supprimé tous les constructeurs et opérateurs de copie mais je vais bien sûr le faire dans la version réelle. Quid des constructeurs et opérateurs de déplacement par contre ?

Merci pour vos retours

[Bousk]

Salut,

ça a l'air pas mal, et si en plus ça marche c'est gagné ^^
Perso je supprimerais les constructeurs et assignations par copie au cas où.
Dans l'utilisation, quitte à avoir un objet temporaire, j'opterais surement pour des appels chaînés : bus.startOperation().work().send(42); (la syntaxe with du Python est terrible pour ça, j'adorerais avoir un équivalent en C++). Tout ce que tu as besoin de changer est de retourner un LockedSpiBus& via *this dans ses fonctions, mais ça évite d'avoir à déclarer une variable intermédiaire et d'introduire un scope pour limiter sa portée (ce qui n'apparait pas dans ton exemple et tu te retrouves avec une vieille locked_bus qui traîne) et profiter du autounlock.
De la même manière, je supprimerais les interfaces lock & unlock du Bus qui ne serviront plus afin de forcer à utiliser cette syntaxe.

[dalfab]

Bonjour,

Dans les processus temps réel sur lesquels je travaille, le mutex n'est pas un moyen "propre", c'est un palliatif à un défaut de structure.
Si 2 process veulent utiliser la SPI "au même moment", le second est alors bloqué jusqu'à la libération, ce qui empêche un contrôle précis des ressources temporelles.
Dans ce genre de cas, on créé un arbitre général qui utilise un thread unique, il n'y a alors aucun besoin d'un quelconque mutex. Les demandes sont pipelinées par les files d'attentes du thread; Le demandeur peut attendre une réponse immédiate ou demander une réponse ultérieure.

Pour les drivers de bas niveau du type SPI, on a plusieurs barrières d'utilisation
- la première est rendue dès que le message SPI débute (=le driver est prêt à recevoir une autre demande)
- dès que le dernier mot à recevoir est lu, un second niveau se débloque (=le driver peut répondre à l'utilisateur)
- la dernière est rendue dès que l'interruption à transmis le dernier mot (=accès physique autorisé)

[Pyramidev]

Bonjour,

Je ne sais pas si les mutex sont adaptés à ton besoin mais, personnellement, quand je travaille avec des mutex, ma technique préférée est celle-ci :

Exemple avec un logueur thread-safe simplifié :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
#include <functional>
#include <mutex>
#include <shared_mutex>
 
template<class DataT>
class Synchronized
{
public:
	explicit Synchronized(DataT data = DataT{}) :
		m_data{std::move(data)},
		m_mutex{}
	{}
 
	template<class Callable>
	decltype(auto) operator()(Callable&& callable) const
	{
		std::shared_lock myLock{m_mutex};
		return std::invoke(std::forward<Callable>(callable), m_data);
	}
 
	template<class Callable>
	decltype(auto) operator()(Callable&& callable)
	{
		std::unique_lock myLock{m_mutex};
		return std::invoke(std::forward<Callable>(callable), m_data);
	}
 
private:
	DataT                     m_data;
	mutable std::shared_mutex m_mutex;
};
 
#include <string>
 
class Logger {
public:
	explicit Logger(std::string const& filePath);
	std::string getFilePath() const;
	void log(const std::string& message); //!< Open log file if needed.
	bool isOpen() const;
	void close();
private:
	std::string m_filePath;
	// ...
};
 
class ThreadSafeLogger {
public:
	explicit ThreadSafeLogger(std::string const& filePath) :
		m_syncLogger{Logger{filePath}}
	{}
 
	template<class Callable>
	decltype(auto) criticalSection(Callable&& callable) const
	{
		return m_syncLogger(std::forward<Callable>(callable));
	}
 
	template<class Callable>
	decltype(auto) criticalSection(Callable&& callable)
	{
		return m_syncLogger(std::forward<Callable>(callable));
	}
 
	std::string getFilePath() const {
		return m_syncLogger(&Logger::getFilePath);
	}
 
	void log(const std::string& message) {
		m_syncLogger( [&](Logger& logger){logger.log(message);} );
	}
 
private:
	Synchronized<Logger> m_syncLogger;
};

Exemple de code qui utilise ThreadSafeLogger::criticalSection :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
myThreadSafeLogger.criticalSection([](Logger& logger) {
	if(logger.isOpen())
		logger.close();
	sendLogFileToDeveloperByMail(logger.getFilePath());
});

[Pyramidev]

Je me suis rendu compte que mon modèle de classe Synchronized avait un défaut : quand on appelait operator() sur un objet mutable de type Synchronized<X>, c'était par défaut la surcharge non const de Synchronized::operator() qui était appelée, même quand le callable en paramètre pouvait manipuler un X const&.
On avait alors un appel à std::shared_mutex::lock alors qu'il suffisait d'appeler std::shared_mutex::lock_shared, qui évite de bloquer les autres lecteurs.
L'appelant pouvait quand même appeler la surcharge const de Synchronized::operator() en utilisant std::as_const, mais c'était dommage de devoir y penser.

Voici une nouvelle version de mon modèle de classe Synchronized qui privilégie la surcharge const de Synchronized::operator() :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
#include <functional>
#include <mutex>
#include <shared_mutex>
#include <type_traits>
 
template<class DataT>
class Synchronized
{
public:
	explicit Synchronized(DataT data = DataT{}) :
		m_data{std::move(data)},
		m_mutex{}
	{}
 
	template<class Callable>
	decltype(auto) operator()(Callable&& callable) const
	{
		std::shared_lock myLock{m_mutex};
		return std::invoke(std::forward<Callable>(callable), m_data);
	}
 
	template<class Callable, std::enable_if_t<!std::is_invocable_v<Callable&&, DataT const&>, int> = 0>
	decltype(auto) operator()(Callable&& callable)
	{
		std::unique_lock myLock{m_mutex};
		return std::invoke(std::forward<Callable>(callable), m_data);
	}
 
private:
	DataT                     m_data;
	mutable std::shared_mutex m_mutex;
};