[SAMMELTHREAD] Kleine Coding Fragen

Language: C
Guten Tag, ich habe gerade eine Aufgabe mit p_threads. Evt. eine wichtige Information ^^.

Mein Problem gerade ist,

das ich zwei while schleifen habe und die erste funktioniert einwandfrei, bloß bei der zweiten funktioniert mein Programm nicht mehr, wenn dort nix drinnen steht also a++ oder co. ging auch nicht nur wenn gefühlt ein Print passiert?

Ist das ein Bug von unseren Unirechnern oder brauch die schleife irgendetwas?

Funktioniert:

            while (enter[i])
                ;
            while((tickets[i] != 0) && ((tickets[tid] > tickets[i]) || ((tickets[tid] == tickets[i])&&(tid > i))))
            {
                printf("thread waiting\n");
            }

Funktioniert nicht:

            while (enter[i])
                ;
            while((tickets[i] != 0) && ((tickets[tid] > tickets[i]) || ((tickets[tid] == tickets[i])&&(tid > i))))
            {
                //printf("thread waiting\n");
            }

            while (enter[i])
                ;
            while((tickets[i] != 0) && ((tickets[tid] > tickets[i]) || ((tickets[tid] == tickets[i])&&(tid > i))))
                ;

            while (enter[i] || ((tickets[i] != 0) && ((tickets[tid] > tickets[i]) || ((tickets[tid] == tickets[i])&&(tid > i)))))
                ;

Mogly1 Noch schlechter hätte man die Frage glaube wirklich nicht formulieren können.

Was genau möchtest du erreichen.

Erkläre doch mal mehr zu deinem Code. Was genau geht denn nicht? Die printf sind ja ausgeklammert, da wird also nichts ausgegeben. Wieso geht denn das nicht?

Was soll das random Semikolon da? Soll das eine Endlosschleife sein? Dieser Code ist maximal dirty.

Eventuell mehr auf Clean-Code achten...

Kaliber

Oh tut mir leid (wenn man so in Gedanken ist, vergisst man immer, dass ihr ja nicht alles habt wie ich :3).

Also Ausgangspunkt:

Wir sollen mit pthreads eine Simulation bauen. (von Autos die über die Brücke fahren aber immer einzeln)

Im Code sollen wir noch ohne Hardware oder Betriebssystem Unterstützung arbeiten, um kritische Abschnitte zu bearbeiten.

Die Threads generieren sich ganz normal,

hier die lock und unlock Methoden, für den kritischen Abschnitt:

int lock (long tid) {
    int i, max = 0;
    enter[tid] = 1;
    for (i = 0; i < NUM_THREADS; i++) {
        if (max < tickets[i])
        max = tickets[i];
    }
    tickets[tid] = max + 1;
    enter[tid] = 0;
    printf("tid: %ld, my ticket is: %d\n", tid, tickets[tid]);
    for (i = 0; i < NUM_THREADS; i++) {
        if (i != tid) {
            while (enter[i])
                ;
            while((tickets[i] != 0) && ((tickets[tid] > tickets[i]) || ((tickets[tid] == tickets[i])&&(tid > i))))
            {
                printf("thread waiting\n");//ohne das geht es nicht O.o
            }
        }
    } 
    return 0;
}

int unlock (long tid) {
 printf("tid: %ld, unlocked \n", tid);
 tickets[tid] = 0;
 return 0;
}

Die Endlosschleifen sind wichtig, da die Threads während des kritischen Abschnittes nichts tun dürfen, da es sonst zu Fehlern kommt.(2 Autos auf der Brücke)

Das "random Semikolon" ist doch dafür, dass die Endlosschleife funktioniert oder nicht? Wie schreibt man das sonst besser?

So nun das Problem:

Wenn ich das

printf("thread waiting\n");

entferne, kommt es zu einem Deadlock. Warum weiß ich aber nicht.

(es ergibt für mich auch keinen Sinn)

Kaliber (Wieso ist der Code maximal dirty? Wie schreibt man das besser?)

Könntest du die ganze Datei teilen, an der du gerade arbeitest?

Gerade in C-Sprachen ist es wichtig den Gesamtkontext zu sehen und ehrlich gesagt verwirrt mich der Aufbau ein wenig.

Andosius

Ja hier meine datei:
NUM_THREADS ist auf 5 welches aber für beliebige gehen soll.

crosses ist auf 10 welche aber eigtl. auf 100.000 mind. seien sollen. (aber durch den print, dauert das etwas. Weshalb ich es auf 10 gestelt habe)

#define _DEFAULT_SOURCE
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <pthread.h>
#include <time.h>

#define NUM_THREADS   5
#define sle 0.01
#define crosses 10

clock_t start_t, end_t;
char enter[NUM_THREADS]; // init with 0
int tickets[NUM_THREADS]; // init with 0
int bridge_crosses;
int accidents;
int state[NUM_THREADS];

int lock (long tid) {
    int i, max = 0;
    enter[tid] = 1;
    for (i = 0; i < NUM_THREADS; i++) {
        if (max < tickets[i])
        {
            max = tickets[i];
        }
    }
    tickets[tid] = max + 1;
    enter[tid] = 0;
    printf("tid: %ld, my ticket is: %d\n", tid, tickets[tid]);
    for (i = 0; i < NUM_THREADS; i++) {
        if (i != tid) {
            while (enter[i])
                ;
            while((tickets[i] != 0) && ((tickets[tid] > tickets[i]) || ((tickets[tid] == tickets[i])&&(tid > i))))
            {
                printf("thread waiting\n");//ohne das geht es nicht O.o
            }
        }
    } 
    return 0;
}

int unlock (long tid) {
 printf("tid: %ld, unlocked \n", tid);
 tickets[tid] = 0;
 return 0;
}


void *bridge_scenario (void *threadid)
{
  long tid;
  tid = (long) threadid;
  printf ("Hello World! It's me, thread #%ld !\n", tid);
  while (bridge_crosses < crosses) {
    if (state[tid] == 0 || state[tid] == 3) {
        
        // try to access critical section
        lock(tid);
        printf("tid: %ld doing stuff\n", tid);
        state[tid] += 1;
        for (int i = 0; i < NUM_THREADS; i++) {
            if (i == tid) {
                continue;
            }
            if (state[i] == 1 || state[i] == 4){
                accidents += 1;
                //printf("KOLLISION\n");
                break;
            }
        }
        sleep(sle);
        state[tid] += 1;
        unlock(tid);
        bridge_crosses += 1;
    }
    else if (state[tid] == 2) {
        //sleep(sle);
        state[tid] += 1;
    }
    else {
        //sleep(sle);
        state[tid] = 0;
    }
  }

  pthread_exit (NULL);
}

int main()
{
  start_t = clock();    
  pthread_t threads[NUM_THREADS];
  int rc;
  long t;

  // init data
  bridge_crosses = 0;
  accidents = 0;

  for (t = 0; t < NUM_THREADS; t++) {
    state[t] = 0;
     printf ("In main: creating thread %ld\n", t);
     rc = pthread_create(&threads[t], NULL, bridge_scenario, (void *)t);
     if (rc) {
        printf ("ERROR; return code from pthread_create () is %d\n", rc);
        exit (-1);
     }
  }

  // joining threads
  for (t = 0; t < NUM_THREADS; t++) {
    pthread_join(threads[t], NULL);
  }

  // output
  end_t = clock();
  double total_t = (double)(end_t - start_t) / CLOCKS_PER_SEC;
  printf ("bridge crosses: %d\n accidents: %d \n time: %f \n", bridge_crosses, accidents, total_t);

  /* Last thing that main() should do */
  pthread_exit (NULL);
  return 0;
}

meine Makefile:

CC = gcc
CFLAGS = -std=c11 -Wall -Wextra -pedantic -O2 -g -pthread
#CPPFLAGS = -MMD -MF $*.d #activate only if you want extra output as .d files
# TODO: edit BIN with your target binary (e.g. exercise01)
BIN  = uebung02
OBJS = $(BIN).o

all: $(BIN) 

# uncomment this if you want object files
#$(BIN): $(OBJS)

run: $(BIN)
    ./$(BIN)

# define test cases here
test1: $(BIN)
    ./$(BIN) 5
test2: $(BIN)
    ./$(BIN) 10

# run all testcases
test: test1 test2

clean:
    @$(RM) -v $(BIN) *.o *.d

.PHONY: clean all

Output so:

output ohne:

printf("thread waiting\n");//ohne das geht es nicht O.o

und dann ist ein deadlock (also passiert nichts mehr...

Mogly1 dann versuch ich mal etwas Licht ins Dunkel zu bringen.

Wieso ist das bad code?

Okay, nun zum Rest.

Ich verstehe nicht, wofür du die status Variable benötigst. Ich verstehe weiterhin nicht, wofür du die enter Variable brauchst.

Weiterhin verstehe ich nicht, wofür du die "Kollision Variable" brauchst. Der Plan ist doch, dass sowas nicht stattfinden kann, oder etwa nicht?

Wieso dann nicht z.B. einfach so lösen?

//Edit: Funktioniert nicht, außer als wenn ticket eine atomare Variable ist.

#define _DEFAULT_SOURCE
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <pthread.h>
#include <time.h>

#define NUM_THREADS   5
#define sle 0.01
#define crosses 100

clock_t start_t, end_t;

volatile long ticket;
int bridge_crosses;

void lock (long tid) {
    while(ticket != -1)
    {
        printf("Thread [%ld] waiting...\n",tid);
        sleep(sle);
    }
    ticket = tid;
}

void unlock (long tid) {
     printf("Thread [%ld] unlocked!\n", tid);
     ticket=-1;
}


void *bridge_scenario (void *threadid)
{
  long tid = (long) threadid;
  printf ("Hello World! It's me, thread #%ld !\n", tid);
  while (bridge_crosses < crosses) {
        lock(tid);
        printf("Car [%ld] crossing the road...\n", tid);
        sleep(sle);
        unlock(tid);
        bridge_crosses += 1;
        sleep(sle);
    }

  pthread_exit (NULL);
}

int main()
{
  start_t = clock();    
  pthread_t threads[NUM_THREADS];
  long t;

  // init data
  bridge_crosses = 0;
  ticket = -1;

  for (t = 0; t < NUM_THREADS; t++) {
     printf ("In main: creating thread %ld\n", t);
     int rc = pthread_create(&threads[t], NULL, bridge_scenario, (void *)t);
     if (rc) {
        printf ("ERROR; return code from pthread_create () is %d\n", rc);
        exit (-1);
     }
  }

  // joining threads
  for (t = 0; t < NUM_THREADS; t++) {
    pthread_join(threads[t], NULL);
  }

  // output
  end_t = clock();
  double total_t = (double)(end_t - start_t) / CLOCKS_PER_SEC;
  printf ("bridge crosses: %d\n time: %f \n", bridge_crosses, total_t);

  /* Last thing that main() should do */
  pthread_exit (NULL);
  return 0;
}

So könntest z.B. auch ganz einfach zählen, wie oft welches Auto gerade drübergefahren ist.

Und es kann so zu keinen Kollisionen kommen.

Der Grund btw, wieso das ohne das printf bei dir nicht funktioniert ist der, dass der Compiler die Variablen und Abfragen wegoptimiert 😄 Auf -O2 denkt der sich, ach, das brauch der nicht, weg damit xD

Damit das nicht passiert, musst du sie volatile setzen, dann lässt der Compiler die Finger davon

Hoffe es hilft etwas.

Kaliber Es lag wirklich an -O2 danke dir :3

Die Status variable ist dafür da, um zu überprüfen, ob 2 Autos quasi im kritischen Abschnitt sind. Somit kann ich die nicht entfernen (so wie die for schleife dazu)

Sonst kann ich das ja nicht kontrollieren.

Also der Code von dir funktioniert nicht, dort fahren mehrere Autos über die Brücke, da der kritische Abschnitt nicht richtig ist gesichert ist. (quasi kein korrekter code)

Ich habe mich an "Multiple Locks with Mutual Access – Lamport" gehalten.

Aber ist ja egal. Mir ging es ja nur darum, wieso die Schleife nicht ging, wenn ich das printf entferne.

Zitat von Mogly1

Also der Code von dir funktioniert nicht, dort fahren mehrere Autos über die Brücke, da der kritische Abschnitt nicht richtig ist gesichert ist

Naja, ich sag mal, eigentlich sollte der genauso funktionieren wie dein Code.

Wieso meinst du, dass er nicht funktioniert?

Das heißt in beiden Fällen kann es zu einer Race-Condition kommen, da die Variablen nicht richtig synchronisiert werden zwischen den Threads.

Dafür sollte man z.B. stdatomic.h oder Mutexes nutzen.

Beides wird hier nicht getan, somit ist das so oder so kritisch...

Zitat von Kaliber

Naja, ich sag mal, eigentlich sollte der genauso funktionieren wie dein Code.

Wieso meinst du, dass er nicht funktioniert?

Nein, das ist leider absolut falsch, du nutzt nur einen normalen Lock mit einer globalen Variable, während mein Code eine Twofold-Lock und Ticketsystem(warteraum) nutzt.

Grund:

Zitat

- Checking and setting the lock variable
is by itself a critical section.
- The simple lock approach does not
meet all requirements for a solution to
protect a critical section.

Ein Lock reicht ungünstigerweise nicht aus, da das Sichern eines kritischen Abschnittes selbst kritisch ist.

Dein Code hat leider Konflikte im kritischen Abschnitt Output:
(Threads: 20 Brückenüberfahrten: 10.000)

(Threads: 5 Brückenüberfahrten: 100.000)

Mein Code (Ohne Konflikte):

(Threads: 20 Brückenüberfahrten: 10.000)

(Threads: 5 Brückenüberfahrten: 100.000)

Aufgrund Lage der Zeit habe ich das nicht weiter hochskaliert, aber ich kann dir sagen, dass "Multiple Locks with Mutual Access – Lamport"

Zitat

The multiple locks with mutual access (Lamport) approach meets all requirements for a solution to protect a critical section.

Zitat von Kaliber

Das heißt in beiden Fällen kann es zu einer Race-Condition kommen, da die Variablen nicht richtig synchronisiert werden zwischen den Threads.

Natürlich versucht jeder am schnellsten den lock zu nutzen. Jedoch, durch den Twofold-Lock, warten die anderen Threads bis ein Thread wieder aus dem kritischen Abschnitt ist, dies ist aber nur möglich mit den Twofold-Lock und einem Warte-System. oder "Twofold Lock with Mutual Precedence"

Zitat von Kaliber

Dafür sollte man z.B. stdatomic.h oder Mutexes nutzen.

Wie erwähnt, sollen wir das ohne Hardware- oder Betriebssystemunterstützung lösen.

Mit Mutex ist es ja ganz simpel :3

Clean Code ist sehr wichtig, aber Kaliber du musst darauf achten, dass dein Clean Code nicht die Korrektheit ändert. Was bei deinem Code bedauerlicherweise der Fall ist.

Dein Cleaner Code hätte jetzt mein Code verbuggt.

Am Ende wurde meine Frage beantwortet von dir, also danke ich dir nochmals dafür.

Zitat von Mogly1

Ein Lock reicht ungünstigerweise nicht aus, da das Sichern eines kritischen Abschnittes selbst kritisch ist.

Yes mb, es war nicht atomar. Sry, es ist 10 Jahre her, dass ich manuell so sections sichern musste...

Genau aus dem Grund macht das auch keiner manuell xD

Zitat von Mogly1

nicht die Korrektheit ändert

Hat ja nichts mit Clean Code zu tun. Was eine Troll Ergänzung.

Zitat von Kaliber

Hat ja nichts mit Clean Code zu tun. Was eine Troll Ergänzung.

Meine Aussage war doch nur das Clean Code wichtig ist und du da recht hast.

Jedoch nur als hint ist dein Clean Code nicht korrekt und darauf muss man achten, dass wollte ich damit nur sagen.

Mir ging es nur darum, du meintest mein code ist dirty, ist ja auch richtig mein Code kann Cleaner geschrieben werden keine Fragen.

Aber wenn du Clean Code vorschlägst, sollte er auch korrekt bleiben. Darum ging es mir nur, ich habe mich bei der Aussage nur auf deinen Clean Code bezogen, nicht auf Clean Code allgemeinen.

Damit wünsche ich noch einen schönen Tag