4. Zwei Threads - Eine globale Variable inkrementieren

Zwei Threads - Eine globale Variable inkrementieren

  • R

    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);
}
    0
  • D

    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.

    0
  • ?

    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.

    0
  • D

    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?

    0
  • R

    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
...
    0
  • D

    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.

    0
  • R

    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?

    0
  • D

    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.

    0
  • R

    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.

    0
  • D

    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.

    0
  • R

    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. 😉

    0
  • D

    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.

    0
  • ?

    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; } }
    0
  • D

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

    0
  • R

    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!

    0
  • ?

    dachschaden schrieb:

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

    LOL

    Die Aufgabe besteht daraus, zwei Threads abwechselnd eine globale Variable (Increment) jeweils um 1 zu inkrementieren."

    Da steht nix von OpenMP verboten.

    0
  • D
     

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

    Fehler, du greift hier auf Elemente zu, die du nicht reserviert hast. Dass das funktioniert ist reines Glück.

    Du musst das nächste Element, welches du freigibst, ordentlich ermitteln:

    void*zaehlen(void*cr)
{
    uintptr_t id = (uintptr_t)cr,id_next = ((id + 1) == THREADS ? 0 : id + 1);
    while(1)
    {
        sem_wait(&sem[id]);
        if(Increment == LOOP)
            break;

        ++Increment;
        printf("Thread: %lx|%lx|\t%d\n",id,id_next,Increment);
        sem_post(&sem[id_next]);
    }
    sem_post(&sem[id_next]);
    return NULL;
}

    So greifst du niemals auf ungültige Elemente zu.

    versionsnummer schrieb:

    Da steht nix von OpenMP verboten.

    Das Beispiel zielt darauf ab, dass derjenige, der die Aufgaben erhalten hat, begreift, was es mit Synchronizität zu tun hat und wie man kritische Sektionen erkennt und mit ihnen umgeht. Dein OpenMP-Beispiel umgeht diesen Lerneffekt. Damit hast du das Thema verfehlt. Ich bleibe dabei, 6, setzen.

    Außerdem: warum überhaupt C++ und OpenMP? Ich dachte, die haben jetzt diese schmucken Futures, die vom Standard unterstützt werden (und die in der Realität langsamer sind als OpenMP, aber die Ausrede lasse ich nicht gelten).

    EDIT: Nee, die sind langsamer, nicht schneller. Einfach deswegen, weil die sich entscheiden können, es eben doch nicht async zu machen, sondern nur deferred, und weil C++ eh besondere Flaschenhälse. Stattdessen muss man dann 3rd-Party-Zeugs verwenden oder sein Threadpooling doch selbst machen.

    0
  • R

    dachschaden schrieb:

    So greifst du niemals auf ungültige Elemente zu.

    Sehr cool, vielen Dank!

    dachschaden schrieb:

    id_next = ((id + 1) == THREADS ? 0 : id + 1);

    Die Operationen sind mir auch neu.
    Ich weise id_next also einem Wert zu, wenn (id + 1) gleich einer Zahl aus der Schleife von 0 bis THREADS ist, wobei id immer um 1 erhöht wird?

    0
  • D

    Addiere auf id temporär 1, prüfe, ob dies gleich THREADS ist (also ein inbound-Check). Wenn dem der Fall ist, sind wir der Letzte in der Liste, und den Semaphore, den wir freigeben wollen, ist dann der erste - der hat den Index 0. Wenn wir nicht der Letzte in der Liste sind, dann nehmen wir einfach das nächste Element. 0 oder das nächste Element ( id + 1 ) wird dann zugewiesen.

    id selbst wird nicht um 1 erhöht. Höchstens wird der Wert von id genommen, darauf 1 addiert, und dann in der Berechnung verwendet. Hätte man auch hierdurch ersetzen können:

    uintptr_t id = (uintptr_t)cr;
uintptr_t id_next = id + 1;
if(id_next == THREADS)
    id_next = 0;

    Aber damit könnte ich dann nicht mehr sagen, dass ich auf unter 45 LOC komme. 🙂

    0
  • R

    dachschaden schrieb:

    Addiere auf id temporär 1, prüfe, ob dies gleich THREADS ist (also ein inbound-Check). Wenn dem der Fall ist, sind wir der Letzte in der Liste, und den Semaphore, den wir freigeben wollen, ist dann der erste - der hat den Index 0. Wenn wir nicht der Letzte in der Liste sind, dann nehmen wir einfach das nächste Element. 0 oder das nächste Element ( id + 1 ) wird dann zugewiesen.

    id selbst wird nicht um 1 erhöht. Höchstens wird der Wert von id genommen, darauf 1 addiert, und dann in der Berechnung verwendet. Hätte man auch hierdurch ersetzen können:

    uintptr_t id = (uintptr_t)cr;
uintptr_t id_next = id + 1;
if(id_next == THREADS)
    id_next = 0;

    Das leuchtet mir ein! Ich bedanke mich nochmal für die Hilfe! 🙂

    0
Anmelden zum Antworten