Zwei Threads - Eine globale Variable inkrementieren

rem0ve schrieb:

Mein Gedanke: Dass ich den einen Thread nach dem inkrementieren in sleep versetze, den anderen dann inkrementieren lasse und dann wechsel (1. Thread Wake, 2. Sleep) usw.

Wo ist da dann noch der Sinn dabei, Threads zu benutzen.
Lass dein System entscheiden, wann welcher Thread abgearbeitet wird.
Dein Beispiel macht übrigens nicht viel Sinn und geht an der Aufgabenstellung vorbei.

rem0ve

SeppJ schrieb:

Mit "Wechseln" meinst du, ein Thread soll nur dann den Wert erhöhen, wenn ein anderer Thread zuvor den Wert erhöht hat? Falls ja, dann ist das genau die Bedingung, die du umsetzen musst.

Damit meine ich, dass wenn zwei Threads (td1, td2) gegeben sind, soll erst td1 den Wert erhöhen, dann td2, dann wieder td1, usw.

Deine Mutexe sind genau falsch. In der glibc, die du wahrscheinlich benutzt, ist printf bereits garantiert threadsicher und sogar synchronisiert. Ob ein aber ein Integer ein atomarer Datentyp ist, ist nicht unbedingt gegeben.

Bin erst seit kurzem dabei, C zu lernen. Das habe ich tatsächlich nicht gewusst. Vielen Dank dafür! Habe meinen Code entsprechend abgeändert.

Kater123 schrieb:

Dein Beispiel macht übrigens nicht viel Sinn und geht an der Aufgabenstellung vorbei.

Ein Beispiel macht keinen Sinn. Wenn schon, ergibt es keinen. :p
Ob der Code / die Aufgabe Sinn ergibt, ist Nebensache. An der Aufgabenstellung kann ich leider nichts ändern.

Hier mein etwas abgeänderter Code. Leider komme ich nicht so weit, dass die Threads sich direkt hintereinander (wie beschrieben) abwechseln.

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>

#define LOOP 1500

int Increment = -1000;

pthread_mutex_t counter_mutex = PTHREAD_MUTEX_INITIALIZER;

void *zaehlen(void *cr) {

	int i;
	for(i = 0; i < LOOP; i++)
		{
			pthread_mutex_lock(&counter_mutex);

			Increment = Increment + 1;

			pthread_mutex_unlock(&counter_mutex);

			printf("Thread: %u |\t%d\n", (unsigned int)pthread_self(), Increment);

		}

	return(NULL);
}

int main(void)
{
	pthread_t td1, td2;

	pthread_mutex_init(&counter_mutex, NULL);

	pthread_create(&td1, NULL, zaehlen, NULL);
	pthread_create(&td2, NULL, zaehlen, NULL);

	pthread_join(td1, NULL);
	pthread_join(td2, NULL);

	return(0);
}

#EDIT

Habe es geschafft! Danke an SeppJ!

Habe es mit Bedingungsvariablen (pthread_cond) gelöst. Mein Gedanke war also gar nicht so daneben mit Wake und Sleep. Mir haben nur die passenden Funktionen in C gefehlt.

Somit kann der Thread als geschlossen angesehen werden. Allen anderen wünsche ich noch einen schönen Abend!

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <unistd.h>

#define LOOP 1500

int Increment = -1000;

pthread_mutex_t counter_mutex = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t cond = PTHREAD_COND_INITIALIZER;

void *zaehlen(void *cr) {

	int i;
	for(i = 0; i < LOOP; i++) {
			pthread_mutex_lock(&counter_mutex);
			pthread_cond_wait(&cond, &counter_mutex);

			Increment = Increment + 1;

			pthread_mutex_unlock(&counter_mutex);

			printf("Thread: %u |\t%d\n", (unsigned int)pthread_self(), Increment);

			pthread_cond_signal(&cond);
		}

	return(NULL);
}

int main(void) {
	pthread_t td1, td2;

	pthread_mutex_init(&counter_mutex, NULL);

	pthread_create(&td1, NULL, zaehlen, NULL);
	pthread_create(&td2, NULL, zaehlen, NULL);

	sleep(1);
	pthread_cond_signal(&cond);

	pthread_join(td1, NULL);
	pthread_join(td2, NULL);

	return(0);
}

dachschaden

man pthread_cond_broadcast schrieb:

The pthread_cond_broadcast() and pthread_cond_signal() functions shall have no effect if there are no threads currently blocked on cond.

Theoretisches Szenario: Thread 2 ist nicht blockiert, läuft aber auch nicht - befindet sich im Limbo zwischen { und pthread_mutex_lock .
Thread 1 läuft, hat gerade den Wert inkrementiert, sendet das Signal. Da Thread 2 nicht auf das Signal wartet, hat das Senden keinen Effekt. Thread 1 beginnt eine neue Schleife, bekommt den Lock, wartet auf das Signal. Thread 2 wird aktiviert, kann den Lock auf den Mutex nicht bekommen -> Deadlock.

Das einzige, was dich hier rettet, ist das printf , weil Ausgaben (seien es Konsole oder Datei) superlangsam sind. Dadurch hast du eine Verzögerung im Code, welcher es wahrscheinlich macht, dass in der Zwischenzeit auch mal der zweite Thread drankommt.

Warum nimmst du nicht zwei binäre Semaphoren? Die haben den Vorteil, dass der Thread, der den Lock bekommen hat, ihn nicht wieder freigeben muss. Thread 1 lockt Semaphore 1, inkrementiert den Wert, gibt Semaphore 2 frei. Thread 2 lockt Semaphore 2, inkrementiert den Wert, gibt Semaphore 1 frei. Und so weiter und so fort.

EDIT: Und lass man die Smilies sein, die lenken hier nur unnötig ab.

EDIT 2: Da hier Threads mit der pthread-Bibliothek verwendet werden, schlage ich vor, den Thread in das Linux/Unix-Unterforum zu verschieben.

rem0ve

dachschaden schrieb:

Theoretisches Szenario: Thread 2 ist nicht blockiert, läuft aber auch nicht - befindet sich im Limbo zwischen { und pthread_mutex_lock .
Thread 1 läuft, hat gerade den Wert inkrementiert, sendet das Signal. Da Thread 2 nicht auf das Signal wartet, hat das Senden keinen Effekt. Thread 1 beginnt eine neue Schleife, bekommt den Lock, wartet auf das Signal. Thread 2 wird aktiviert, kann den Lock auf den Mutex nicht bekommen -> Deadlock.

Nach >15 Testläufen kam es tatsächlich zum beschriebenen Szenario.

dachschaden schrieb:

Warum nimmst du nicht zwei binäre Semaphoren? Die haben den Vorteil, dass der Thread, der den Lock bekommen hat, ihn nicht wieder freigeben muss. Thread 1 lockt Semaphore 1, inkrementiert den Wert, gibt Semaphore 2 frei. Thread 2 lockt Semaphore 2, inkrementiert den Wert, gibt Semaphore 1 frei. Und so weiter und so fort.

Kompilierung läuft ohne Beanstandung, jedoch wieder das Problem -> nicht abwechselnd nach inkrementieren. Habe ich was falsch verstanden, was Semaphoren angeht?

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <unistd.h>
#include <semaphore.h>

#define LOOP 1500

int Increment = -1000;

sem_t semaphore;

void *zaehlen(void *cr) {

	int i;
	for(i = 0; i < LOOP; i++) {
			sem_wait(&semaphore);

			Increment = Increment + 1;

			printf("Thread: %u |\t%d\n", (unsigned int)pthread_self(), Increment);

			sem_post(&semaphore);
		}

	return(NULL);
}

int main(void) {
	pthread_t td1, td2;

	sem_init(&semaphore, 0, 1);

	pthread_create(&td1, NULL, zaehlen, NULL);
	pthread_create(&td2, NULL, zaehlen, NULL);

	pthread_join(td1, NULL);
	pthread_join(td2, NULL);

	sem_destroy(&semaphore);

	return(0);
}

dachschaden

rem0ve schrieb:

Nach >15 Testläufen kam es tatsächlich zum beschriebenen Szenario.

Und jetzt stell' dir mal eine Produktivumgebung vor, wo das Programm immer relativ unreproduzierbar abschmiert, weil so eine Race-Condition auftritt ... deswegen muss man immer vorsichtig sein, wenn man mit Multi-Threading arbeitet.

rem0ve schrieb:

Habe ich was falsch verstanden, was Semaphoren angeht?

*hust*

dachschaden schrieb:

Warum nimmst du nicht zwei binäre Semaphoren?

rem0ve

dachschaden schrieb:

Und jetzt stell' dir mal eine Produktivumgebung vor, wo das Programm immer relativ unreproduzierbar abschmiert, weil so eine Race-Condition auftritt ... deswegen muss man immer vorsichtig sein, wenn man mit Multi-Threading arbeitet.

Stimmt, das sollte man vermeiden.

dachschaden schrieb:

*hust*

Wer lesen kann...

Ich stehe gerade total auf dem Schlauch. Nun ist das Problem, dass immer wieder in "S2 freigegeben" gefahren wird, da nach dem ersten Ablauf S2 immer wieder auf 0 und wieder auf 1 gesetzt wird. Ich sehe gerade den Wald vor lauter Bäume nicht...

void *zaehlen(void *cr) {

	int i;

	for(i = 0; i < LOOP; i++) {

			sem_getvalue(&semaphore1,&S1);
			if((S1 == 1)) {
				sem_wait(&semaphore1);
				printf("\nS1 geblockt!\n");
			}
			printf("Value1: %d\n",S1);

			sem_getvalue(&semaphore2,&S2);
			if((S2 == 1)) {
				sem_wait(&semaphore2);
				printf("\nS2 geblockt!\n");
			}
			printf("Value2: %d\n",S2);

			Increment = Increment + 1;

			printf("Thread: %u |\t%d\n", (unsigned int)pthread_self(), Increment);

			sem_getvalue(&semaphore1,&S1);
			if((S1 == 0)) {
				sem_post(&semaphore2);
				printf("\nS2 freigegeben!!!\n");
			}
			printf("Value1: %d\n",S1);

			sem_getvalue(&semaphore2,&S2);
			if((S2 == 0) ) {
				sem_post(&semaphore1);
				printf("\nS1 freigegeben!!!\n");
			}
			printf("Value2: %d\n",S2);

		}

	return(NULL);
}

dachschaden

rem0ve schrieb:

Ich stehe gerade total auf dem Schlauch. Nun ist das Problem, dass immer wieder in "S2 freigegeben" gefahren wird, da nach dem ersten Ablauf S2 immer wieder auf 0 und wieder auf 1 gesetzt wird. Ich sehe gerade den Wald vor lauter Bäume nicht...

Mal so als Tipp: die Semaphoren in einem Array zu verwalten und den Threads beim Erzeugen eine ID zu verpassen kann die Programmlogik erheblich vereinfachen. Und den ! -Operator kannst du auch zum "Toggeln" von Werten verwenden.

EDIT: Bonuspunkte, wenn du es schaffst, das Programm so umzuschreiben, dass man nur eine Konstante ändern muss, um damit die Anzahl der erzeugten Threads zu ändern und damit jeder Thread nacheinander den Wert inkrementiert, ohne dass ein Deadlock passiert oder noch weitere Teile des Codes umgeschrieben werden müssen. Ist in unter 60 Zeilen zu schaffen.

ich hab das damals (tm) so gemacht, dass ich einfach selbst eine sperrvariable erstellt und die threads immer wieder schlafen geschickt habe, bis die sperrvariable den wert der thread-id hatte.

wenn du eine struktur mit zeigern erstellst, kannst du auch bequem unbegrenzt daten an threads übergeben, ohne globale variablen zu verwenden.

dachschaden

HansKlaus schrieb:

ich hab das damals (tm) so gemacht, dass ich einfach selbst eine sperrvariable erstellt und die threads immer wieder schlafen geschickt habe, bis die sperrvariable den wert der thread-id hatte.

Faustregel: wenn du in deinem Threading-Code sleep verwenden musst, machst du mit relativ hoher Wahrscheinlichkeit was grundlegend falsch.

HansKlaus schrieb:

wenn du eine struktur mit zeigern erstellst, kannst du auch bequem unbegrenzt daten an threads übergeben, ohne globale variablen zu verwenden.

Was hat das mit dem Problem zu tun?

rem0ve

dachschaden schrieb:

Mal so als Tipp: die Semaphoren in einem Array zu verwalten und den Threads beim Erzeugen eine ID zu verpassen kann die Programmlogik erheblich vereinfachen. Und den ! -Operator kannst du auch zum "Toggeln" von Werten verwenden.

EDIT: Bonuspunkte, wenn du es schaffst, das Programm so umzuschreiben, dass man nur eine Konstante ändern muss, um damit die Anzahl der erzeugten Threads zu ändern und damit jeder Thread nacheinander den Wert inkrementiert, ohne dass ein Deadlock passiert oder noch weitere Teile des Codes umgeschrieben werden müssen. Ist in unter 60 Zeilen zu schaffen.

Habe es zum Teil abgeändert (Anzahl der Threads durch die Marke THREADS, Semaphoren + Threads in Arrays).

Problem besteht weiterhin. Code + Ausgabe im Anschluss. Es ärgert mich wirklich, dass ich einfach nicht dahinter komme. Ich habe es schon versucht nach dem Prinzip -> Wait[0], Signal [1], INK, Signal[0], Wait[1] -> was ja eigentlich der Algorithmus dahinter sein sollte? Eine Art Ampelschaltung. Funktioniert nur leider nicht. Leider ist mein Wissen über Threads (insbesondere in C) sehr begrenzt.

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <semaphore.h>

#define LOOP 5
#define THREADS 10
#define SEMAPHOREN 2

int Increment = -1000;
int switches[2];

sem_t semaphoren[2];

void *zaehlen(void *cr) {

	int i;

	for(i = 0; i < LOOP; i++) {
			sem_getvalue(&semaphoren[0],&switches[0]);
			if(switches[0]) {
				sem_wait(&semaphoren[0]);
				printf("Semaphore[0] geblockt\n");
			}

			sem_getvalue(&semaphoren[1],&switches[1]);
			if(switches[1] && !switches[0]) {
				sem_wait(&semaphoren[1]);
				printf("Semaphore[1] geblockt\n");
			}

			Increment = Increment + 1;

			printf("Thread: %u |\t%d\n", (unsigned int)pthread_self(), Increment);

			sem_getvalue(&semaphoren[0],&switches[0]);
			if(!switches[0]) {
				sem_post(&semaphoren[1]);
				printf("Semaphore[1] freigegeben\n");
			}

			sem_getvalue(&semaphoren[1],&switches[1]);
			if(!switches[1]) {
				sem_post(&semaphoren[0]);
				printf("Semaphore[0] freigegeben\n");
			}
		}
	return(NULL);
}

int main(void) {
	pthread_t threads[THREADS];

	sem_init(&semaphoren[0], 0, 1);
	sem_init(&semaphoren[1], 0, 1);

	int i;
	for(i = 0; i < THREADS; i++) {
		pthread_create(&threads[i], NULL, zaehlen, NULL);
		pthread_join(threads[i], NULL);
	}

	for(i = 0; i < SEMAPHOREN; i++) {
		sem_destroy(&semaphoren[i]);
	}

	pthread_exit(NULL);

	return(0);
}

Semaphore[0] geblockt
Thread: 684812032 |	-999
Semaphore[1] freigegeben
Semaphore[1] geblockt
Thread: 684812032 |	-998
Semaphore[1] freigegeben
Semaphore[1] geblockt
Thread: 684812032 |	-997
Semaphore[1] freigegeben
Semaphore[1] geblockt
Thread: 684812032 |	-996
Semaphore[1] freigegeben
Semaphore[1] geblockt
Thread: 684812032 |	-995
Semaphore[1] freigegeben
Semaphore[1] geblockt
Thread: 684812032 |	-994
Semaphore[1] freigegeben
Semaphore[1] geblockt
...

dachschaden

Junge ...

Lass sem_getvalue , das bringt dich nicht weiter, und lass die Threads wissen, um was für einen Thread es sich handelt. Dann weißt du, auf welchen Semaphore du wartest, und welchen Semaphore (nämlich den nächsten, a.k.a. nicht deinen) freigeben willst. Über die ID als Index greifst du auf das Semaphore-Array zu (deswegen sollst du die überhaupt erst als Array verwalten).

Es sollte nur einen einzigen if -Block geben, und in dem prüfst du nur, ob, nachdem du den Lock bekommen hast, du das Limit erreicht hast. Ansonsten ist der Code des kompletten Programms if -Block-frei.

Deine Thread-Erstellung ist fehlerhaft, du kannst nicht einen Thread erzeugen und dann sofort darauf warten, weil der zweite Thread zu diesem Zeitpunkt nicht erzeugt wurde - du brauchst separate Schleifen dafür, eine zum Erstellen der Threads, eine zum joinen. Und du willst nur einen einzigen Semaphore aktiv haben. Alle anderen Semaphoren sollen von Anfang an gelockt sein - deine sem_init s lassen aber alle Threads durch. Was du willst, ist:

1. Thread 1 startet
2. wartet auf Lock für Semaphore 1 (passiert sofort, weil du den ersten Semaphore auf 1 gesetzt hast)
3. inkrementiert
4. gibt Lock für Semaphore 2 (!) frei
5. siehe 2

1. Thread 2 startet
2. wartet auf Lock für Semaphore 2 (passiert nicht sofort, weil der Semaphore auf 0 gesetzt wurde)
3. inkrementiert
4. gibt Lock für Semaphore 1 (!) frei
5. siehe 2

Der einzige Unterschied sind die Indizes, und auf die kannst du mit ein bisschen nachdenken - und der Thread-ID - ganz alleine kommen.

Was macht euer Prof/Ausbilder eigentlich den ganzen Tag, sich die Eier schaukeln?

EDIT: Revidiere meine Aussage, das Programm lässt sich sogar auf 50 Zeilen zusammenpacken.

EDIT 2: Sogar auf unter 45.

rem0ve

dachschaden schrieb:

lass die Threads wissen, um was für einen Thread es sich handelt. Dann weißt du, auf welchen Semaphore du wartest, und welchen Semaphore (nämlich den nächsten, a.k.a. nicht deinen) freigeben willst. Über die ID als Index greifst du auf das Semaphore-Array zu (deswegen sollst du die überhaupt erst als Array verwalten).

Mit "wissen lassen" und "die ID" meinst du die TID vom Thread, welche ich mit pthread_self() aufrufen kann? Diese kann ich doch nicht selber bestimmen und sind ellenlang. Wie soll ich mittels dieser ID ein Index für ein Array erstellen?

dachschaden

rem0ve schrieb:

Mit "wissen lassen" und "die ID" meinst du die TID vom Thread, welche ich mit pthread_self() aufrufen kann? Diese kann ich doch nicht selber bestimmen und sind ellenlang. Wie soll ich mittels dieser ID ein Index für ein Array erstellen?

dachschaden schrieb:

den Threads beim Erzeugen eine ID zu verpassen kann die Programmlogik erheblich vereinfachen

Du hast einen Parameter, dem du einem Thread übergeben kannst. Der ist deine eigene ID. Mit der kannst du arbeiten.

rem0ve

dachschaden schrieb:

Du hast einen Parameter, dem du einem Thread übergeben kannst. Der ist deine eigene ID. Mit der kannst du arbeiten.

Ich wüsste nicht, bei welchem Parameter ich beim erstellen eine eigene ID auswählen könnte.

int pthread_create (pthread_t *th,
                    pthread_attr_t *attr,
                    void *(*start_routine)(void*),
                    void *arg);

Verzeih mir meine Unwissenheit.

dachschaden

Manpage?

man pthread_create schrieb:

The pthread_create() function starts a new thread in the calling process.
The new thread starts execution by invoking start_routine(); arg is passed as the sole argument of start_routine()

IOW RTFM.

size_t i;
for(i = 0;i < THREADS;i++)
    pthread_create(&tids[i],NULL,zaehlen,(void*)(uintptr_t)i);

Da hast du deine custom thread ID.

rem0ve

dachschaden schrieb:

uintptr_t

Das geht weit über den Stoff hinaus, den wir bisher hatten. Vorerst ist für mich das Thema abgeschlossen und ich nehme die "Deadlocks" erstmal hin, da wir beispielsweise auch keine Semaphoren hatten.
Ich bedanke mich für die Hilfe. Sobald ich wieder Zeit für das Programm habe, setze ich mich dran und beende es, damit deine Hilfe nicht umsonst war.

dachschaden

Das ist eine verdammte Typumwandlung, die du in deinem printf bereits machst, und die dem Compiler lediglich sagen soll, dass er die Schnauze zu halten hat, wenn ich ihm eine Ganzzahl gebe und er aber einen Zeiger erwartet. Die kannst du auch wegnehmen, aber dann hast du halt Warnungen.

dachschaden schrieb:

Ist in unter 60 Zeilen zu schaffen. [...]
EDIT 2: Sogar auf unter 45.

unter 45? Ich mach's in 10 ...

#include <omp.h>
#include <iostream>
main(){
  int n;
  omp_set_num_threads(2);
  #pragma omp parallel for ordered schedule(dynamic)
  for(int i = 0; i < 1000; ++i)
    #pragma omp ordered
    { std::cout << "Thread " << omp_get_thread_num() << ": " << n << std::endl;
      ++n; } }

dachschaden

C++ und OpenMP, nicht C und pthreads.
Thema verfehlt, 6, setzen.

rem0ve

dachschaden schrieb:

Das ist eine verdammte Typumwandlung, die du in deinem printf bereits machst, und die dem Compiler lediglich sagen soll, dass er die Schnauze zu halten hat, wenn ich ihm eine Ganzzahl gebe und er aber einen Zeiger erwartet. Die kannst du auch wegnehmen, aber dann hast du halt Warnungen.

Danke. Nun funktioniert es.

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <semaphore.h>
#include <inttypes.h>

#define LOOP 1500
#define THREADS 2
#define SEMAPHOREN 2

int Increment = -1000;

sem_t mutex[2];

void *zaehlen(void *cr) {

	int i;

	for(i = 0; i < LOOP; i++) {
			sem_wait(&mutex[(uintptr_t)cr]);

			Increment = Increment + 1;
			printf("Thread: %lu |\t%d\n", (uintptr_t)cr, Increment);

			sem_post(&mutex[((uintptr_t)cr)+1]);
			sem_post(&mutex[((uintptr_t)cr)-1]);
		}
	return(NULL);
}

int main(void) {
	pthread_t threads[THREADS];

	sem_init(&mutex[0], 0, 1);
	sem_init(&mutex[1], 0, 0);

	size_t i;
	for(i = 0; i < THREADS; i++) {
		pthread_create(&threads[i], NULL, zaehlen, (void*)(uintptr_t)i);
	}

	for(i = 0; i < THREADS; i++) {
		pthread_join(threads[i], NULL);
	}

	for(i = 0; i < SEMAPHOREN; i++) {
		sem_destroy(&mutex[i]);
	}

	pthread_exit(NULL);

	return(0);
}

Thread: 0 |	1991
Thread: 1 |	1992
Thread: 0 |	1993
Thread: 1 |	1994
Thread: 0 |	1995
Thread: 1 |	1996
Thread: 0 |	1997
Thread: 1 |	1998
Thread: 0 |	1999
Thread: 1 |	2000

#EDIT

Ja, funktioniert nicht für >2 Threads, aber mehr hat die Aufgabenstellung auch nicht verlangt und ich brauche erst mal eine Pause für heute.

#EDIT2

Vielen Dank, dachschaden!