Race Condition - czy coś jeszcze?

Odpowiedz Nowy wątek
2019-07-24 19:52

Rejestracja: 3 lata temu

Ostatnio: 25 minut temu

1

5 wątków puszczanych jednocześnie. Współdzielą jeden obiekt. Każdy z nich najpierw pobiera atrybut współdzielonego obiektu, a następnie zwiększa jego wartość o 1 i nadpisuje. Czynność powtórzona 20 krotnie, więc obiekt zmaglowany przez 5 wątków powinien skończyć z wartością atrybutu większą o 100.
Jak mniemam zdarza się, że np. wątek A pobierze wartość 50 i zanim zapisze ją jako 51 wątek B też pobierze 50 i zapisze 51. Gdy takie przypadki wystąpią to ostateczna wartość atrybutu będzie powiększona o < 100. To mnie nie dziwi. Ale czemu zdarza się, że ostateczna wartość atrybutu jest powiększona o > 100 ?

Drugie pytanie: jak to nareperowac. Dodawanie synchronized do metod nie pomaga...


public class App {

    public static void main(String[] args) throws InterruptedException {
        Counter c1 = new Counter();

        Thread th1 = new Thread(new CounterThread(c1), "thread_1");
        Thread th2 = new Thread(new CounterThread(c1), "thread_2");
        Thread th3 = new Thread(new CounterThread(c1), "thread_3");
        Thread th4 = new Thread(new CounterThread(c1), "thread_4");
        Thread th5 = new Thread(new CounterThread(c1), "thread_5");

        th1.start();
        th2.start();
        th3.start();
        th4.start();
        th5.start();

        th1.join();
        th2.join();
        th3.join();
        th4.join();
        th5.join();

        System.out.println("===");
        System.out.println(c1.getNumber());
        System.out.println("===");

    }

}
public class CounterThread implements Runnable {

    private Counter counter;

    public CounterThread(Counter counter) {
        this.counter = counter;
    }

    @Override
    public void run() {
        for (int i = 0; i<100; i++) {
            synchronized(this) {
                int numberToSet = counter.getNumber();
                System.out.println(Thread.currentThread().getName() + ": " + numberToSet);
                counter.setNumber(numberToSet + 1);
            }

        }
    }
}

public class Counter {

    private int number;
    private boolean isLocked;

    public synchronized int getNumber() {
        this.isLocked = true;
        return number;
    }

    public synchronized void setNumber(int numberToSet) {
        this.number = numberToSet;
        this.isLocked = false;
    }
}
edytowany 2x, ostatnio: Julian_, 2019-07-24 19:53

Pozostało 580 znaków

2019-07-24 19:58

Rejestracja: 13 lat temu

Ostatnio: 2 minuty temu

1

Synchronizujesz na wątku, a nie na liczniku, więc nic dziwnego, że synchronized nie pomaga.
Poza tym zwiększasz licznik 100 razy w każdym wątku, czyli razem 500, o ile dobrze widzę.

co tzn. sycnhronizowac na liczniku? - Julian_ 2019-07-24 20:06

Pozostało 580 znaków

2019-07-24 20:04

Rejestracja: 9 lat temu

Ostatnio: 12 godzin temu

1

Weź podmień może Countera na
private AtomicInteger counter;

Plus to co powiedział @Afish
Ma na myśli, że nie powinieneś robić synchronized(this), tylko synchronized(counter), jeśli już chcesz się trzymać swojej klasy.

edytowany 3x, ostatnio: jackweb, 2019-07-24 20:10

Pozostało 580 znaków

2019-07-24 20:12

Rejestracja: 3 lata temu

Ostatnio: 5 minut temu

Lokalizacja: U krasnoludów - pod górą

2

synchronized ( counter) zamiast synchronized (this) - to drugie jest zupełnie bez sensu.
tak samo jak synchronized na pojedynczych metodach. Działa. Ale nie pomaga rozwiązać problemu. Bo lock musisz mieć na całej operacji increment.


jeden i pół terabajta powinno wystarczyć każdemu

Pozostało 580 znaków

2019-07-24 21:40

Rejestracja: 3 lata temu

Ostatnio: 25 minut temu

0

A co się mogło stać, że czasem ostateczna wartość przekracza wartość liczby iteracji pomnożonej przez wątki?

Pozostało 580 znaków

2019-07-24 21:53

Rejestracja: 3 lata temu

Ostatnio: 5 minut temu

Lokalizacja: U krasnoludów - pod górą

1

Co się mogło stać?

Pewnie oszukałeś.

EDIT:

I jeszcze: System.out.println( (w pętli) poważnie zaburza obserwacje - wywal.
Pętla od 1 do 100 to na increment to raczej śmieszna jest - u mnie te pętle kończą się w czasie niemierzalnym - zanim do odpalenia kolejnego wątku dochodzi.

Rób do miliona lub skomplikuj operacje.
Będziesz miał ciekawszą rzecz do zabawy.


jeden i pół terabajta powinno wystarczyć każdemu
edytowany 3x, ostatnio: jarekr000000, 2019-07-24 22:17

Pozostało 580 znaków

2019-07-24 23:39

Rejestracja: 2 lata temu

Ostatnio: 10 godzin temu

0

W tej chwili to Twój licznik może się cofać, mimo że z pozoru tylko dodajesz :) Pierwszy wątek może odczytać N, pozostałe w międzyczasie ustawią na M > N i przy zapisie pierwszy wątek pomniejsza wartość licznika ustawiając N :D

https://wiki.sei.cmu.edu/conf[...]n+shared+variables+are+atomic

edytowany 1x, ostatnio: damianem, 2019-07-24 23:41

Pozostało 580 znaków

cs
2019-07-25 20:51
cs

Rejestracja: 1 rok temu

Ostatnio: 20 godzin temu

0

W kwestii "dobrej zmiany" programu wywal to synchronized(this) bo jest zbędne. Po drugie zmień klasę Counter:

class Counter {
  private int number;

  public synchronized int getNumber() {
    return number;
  }

  public synchronized void incNumber(){
    number++;
  }
}

wtedy wątek:

@Override
  public void run() {
    for (int i = 0; i < 100; i++) {
      System.out.println(Thread.currentThread().getName() + ": " + counter.getNumber());
      counter.incNumber();
    }
  }

Pozostało 580 znaków

2019-07-25 21:05

Rejestracja: 3 lata temu

Ostatnio: 25 minut temu

0
cs napisał(a):

W kwestii "dobrej zmiany" programu wywal to synchronized(this) bo jest zbędne. Po drugie zmień klasę Counter:

class Counter {
  private int number;

  public synchronized int getNumber() {
    return number;
  }

  public synchronized void incNumber(){
    number++;
  }
}

wtedy wątek:

@Override
  public void run() {
    for (int i = 0; i < 100; i++) {
      System.out.println(Thread.currentThread().getName() + ": " + counter.getNumber());
      counter.incNumber();
    }
  }

o i to jest kolejny przykład któremu się dziwię. Jak puszczę to bez synchrinized to i tak działa jak należy, nie umiem tak tym zatrząść by zadziałało źle

Tak jak napisałem wyżej. Zwiększ limit pętli i wywal ten System.out - jarekr000000 2019-07-25 21:08

Pozostało 580 znaków

cs
2019-07-26 08:29
cs

Rejestracja: 1 rok temu

Ostatnio: 20 godzin temu

2

@Julian_, @jarekr000000: Zamiast zwiększać limit pętli, można też dać szanse na wykonanie przeplotu w sekcji krytycznej, czyli w tym fragmencie, który musi być synchronizowany np.:

class Counter {

    private int number;

    public int getNumber() {
        return number;
    }

    public synchronized incNumber() throws InterruptedException {
        int curr = number;
        System.out.println("Starting inc " + curr);
        this.number++;
        this.number--;
        Thread.sleep(1);
        this.number++;
        System.out.println("Ending inc " + curr +", now value is " + number);
    }
}

Tutaj usunięcie synchronized "psuje" program na tyle, że na 10 uruchomień 8-9 wynik będzie błędny.

Pozostało 580 znaków

2019-07-26 16:28

Rejestracja: 1 rok temu

Ostatnio: 2 miesiące temu

1

juljan, we no przeczytaj jcip, a nie zadawaj lamerskich pytan;)

próbowałem, ale nie wiem o co chodzi. Nie umiem się uczyć zaczynając od teorii. Zawsze na odwrót: najpierw robię, dopiero później zrozumowuję co robię. - Julian_ 2019-07-26 19:49

Pozostało 580 znaków

Odpowiedz

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0