Discussion :

[kain_tn]

Bonjour.


Je me retrouve face à un problème de conception...

J'aimerais développer une classe servant à écrire des données dans un fichier (RAII)
Seulement cette classe peut être accédée par plusieurs threads et je voudrais éviter que plus d'un thread à la fois modifie le fichier.

L'idée que j'avais était de créer une fifo, et de faire en sorte que les threads poussent des données dans la fifo, tandis qu'un autre thread la dépilerait toutes les T périodes et écrirait les données dans le fichier.

L'idée vous paraît-elle viable? Que feriez vous sinon?

Pour ce qui est d'accéder à la fifo, il paraît évident qu'il ne faut pas que tous les threads y accèdent en même temps. Je pensais donc avoir recours à des mutex. J'ai commencé à regarder ceux de boost, mais j'avoue être un peu perdu dans la documentation (en gros, peu de cas pratiques, mais beaucoup de théorie...). Encore une fois, est-ce une bonne idée?

A priori, ma fifo doit être statique pour être accédée de n'importe où non? Est-il préférable de la gérer dans une classe (singleton) ? Est-ce compatible (thread safe) avec ce que je veux faire plus haut?

Bref, je ne sais pas si je suis très clair dans mes explications mais n'hésitez pas à me poser des questions!

Merci d'avance!

[kain_tn]

Bon eh bien en fait j'ai résolu mon problème

Je ne suis pas passé par les mutex de boost mais j'ai créé ma propre classe de mutex et de lock:

En gros, je crée ma classe Writer qui est un singleton:

Code :

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

class Writer: public Singleton<Writer>

Ensuite je crée une classe Mutex, qui contient un booléen mLocked et trois méthodes isLock(), lock() et unlock():

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
 
 class Mutex{
 
	private:
		bool mLocked;
 
	public:
		Mutex()
		:mLocked(false){
		}
 
		bool lock();
		bool unlock();
		bool isLocked()const;
 };

Je crée également une classe Lock, qui prend une référence sur un object Mutex dans son constructeur:

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
 
 class Lock{
 
 	private:
 		Mutex& mMutex;
 
 		void lock(Mutex& mutex){
 			mutex.lock();
 		}
 
 		void unlock(Mutex& mutex){
 			mutex.unlock();
 		}
 
 	public:
 		Lock(Mutex& mutex)
 		:mMutex(mutex){
 			lock(mutex);
 		}
 
 		~Lock(){
 			unlock(mMutex);
 		}
 
 };

1. Le constructeur invoque la méthode lock() sur l'objet Mutex
2. Le destructeur invoque la méthode unlock() sur l'objet Mutex

Ainsi, je m'assure que le Mutex est bien déverrouillé à la mort de l'objet Lock.

Je crée également une classe File, qui fonctionne sur le même principe; à savoir fopen() dans le constructeur, et fclose() dans le destructeur.

Enfin, ma classe Writer contient une fifo (std::queue) ainsi qu'un objet Mutex. Elle contient également les méthodes push() et write().
Ces deux méthodes ne se lancent que si Mutex.isLocked() vaut false.
Elle instancient ensuite un objet Lock

Code :

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

Lock l(mMutex);

Cet objet va donc verrouiller le mutex et le déverrouiller dès que la méthode en cours sera terminée.

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
 
bool push(const std::string& data){
 
			if( !mMutex.isLocked() ){
 
				Lock l(mMutex); // Lock the mutex
                                if( !mMutex.isLocked() )
					return false;
 
				mData.push( data );
				return true;
			}
 
			return false;
		}
 
...

[white_tentacle]

Tu as l'air de t'en être sorti tout seul, félicitations .

Toutefois, j'attire ton attention sur plusieurs choses :
- tu as un peu réinventé la roue, il y a déjà tout ça dans boost.
- comment fais-tu effectivement l'écriture dans le fichier ? Il y a un thread en attente active, ou quelque chose du genre ?
- l'appelant doit à chaque fois vérifier que push a réussi, gérer des retry lui-même.

Le premier point n'est pas très important.

Pour le deuxième, il faudrait que tu passes par une condition variable, avec un réveil du thread chargé d'écrire dans le fichier, uniquement lorsque des données ont été rajoutées dans la queue (logiquement, dans la méthode push).

Pour le troisième, j'ai un peu de mal à suivre l'idée. Tes threads font des choses critiques, et ne peuvent pas être bloqués plus d'un certain temps ? Si ce n'est pas le cas, autant mettre une attente infinie sur le mutex. Vu qu'il ne protège pas une ressource externe, mais seulement la queue, tu es tranquille, sauf erreur de programmation, il sera libéré à un moment ou un autre. Push réussira toujours, et tu n'auras pas à te préoccuper de gérer des réessais dans l'appelant à chaque push.

[kain_tn]

Toutefois, j'attire ton attention sur plusieurs choses :
- tu as un peu réinventé la roue, il y a déjà tout ça dans boost.

Oui je sais, mais le faire moi même m'a aidé à comprendre comment fonctionnent ces composants ainsi que la bibliothèque
De plus, même si les options sont limitées comparées à boost, je ne dépends pas d'une bibliothèque externe.

- comment fais-tu effectivement l'écriture dans le fichier ? Il y a un thread en attente active, ou quelque chose du genre ?

Oui. Il y a un thread dédié à l'écriture. Lorsqu'il se réveille, il dépile la fifo.

- l'appelant doit à chaque fois vérifier que push a réussi, gérer des retry lui-même.

Oui. C'est un des points que je veux améliorer mais chaque chose en son temps (par contre, si tu as une idée... )

Pour le deuxième, il faudrait que tu passes par une condition variable, avec un réveil du thread chargé d'écrire dans le fichier, uniquement lorsque des données ont été rajoutées dans la queue (logiquement, dans la méthode push).

Pour la condition, je ne pense pas que ce soit un problème: en fait j'ai un système de "listener" sur mes "loggers", et je peux donc être prévenu si nécessaire qu'il y a eu un appel à push().

Pour le troisième, j'ai un peu de mal à suivre l'idée. Tes threads font des choses critiques, et ne peuvent pas être bloqués plus d'un certain temps ? Si ce n'est pas le cas, autant mettre une attente infinie sur le mutex. Vu qu'il ne protège pas une ressource externe, mais seulement la queue, tu es tranquille, sauf erreur de programmation, il sera libéré à un moment ou un autre. Push réussira toujours, et tu n'auras pas à te préoccuper de gérer des réessais dans l'appelant à chaque push.

OK, je vois un peu l'idée, mais tu ferais ça comment? Avec un while par exemple? Est-ce que ça ne risque pas de bouffer toutes les ressources du processeur?
Tu verrais un truc comme ça?

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
 
bool push(const std::string& data){
 
	while( true ){
 
		if( !mMutex.isLocked() ){
 
			Lock l(mMutex); // Lock the mutex
			if( !mMutex.isLocked() )
				return false;
 
			mData.push(data);
			return true;
		}else
			sleep(2);
	}
}

Merci d'avoir pris le temps de me lire en tous cas!

[white_tentacle]

Tu n'a pas besoin de while, ni d'attente active. Quand tu demandes à bloquer un mutex, si celui-ci est déjà bloqué, le thread est suspendu, il ne fait rien d'autre qu'attendre que le mutex se libère (et ce, sans consommer de processeur).

Bien sûr, cela n'est vrai que si tu n'as pas mis de timeout à ton lock. Mais le code suivant :

Code :

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

1
2
3
Lock l(mMutex); // Lock the mutex
mData.push(data);
return true;

devrait suffire (à condition, bien sûr, que tu n'aies pas précisé de timeout sur ton lock, auquel cas tu n'es pas garanti que ton mutex est locké après le lock !)

[kain_tn]

Je n'ai pas mis de timeout à mon lock, non
Merci pour tes réponses, ça m'a beaucoup aidé!