Wątki - podstawy

Witam. Próbuję zrozumieć jak działają wątki, w związku z tym testuje różne rzeczy i trafiłem na problem, którego nie potrafię zrozumieć. Zmienna sumaWag przyjmuję wartości, których nie powinna np. 10, 90, liczby ujemne, liczby większe od 230. Z góry dziękuje za pomoc w zrozumieniu dlaczego tak się dzieje.

public class WyciagTest 
{ 
	public static void main(String[] args)
	{
		Thread klient1 = new Klient(70);
		Thread klient2 = new Klient(80);
		Thread klient3 = new Klient(80);
		klient1.start();
		klient2.start();
		klient3.start();
	}
}

public class Klient extends Thread 
{
	private int waga;
	private int id=0;
	private static int NextId=1;
	private static int sumaWag=0;
	
	public Klient(int w)
	{
		this.waga=w;
		this.id=NextId;
		NextId++;
	}

	public void zjazd()
	{
		
		System.out.println("Klient nr: " + id+ "Zjezdza z górki");
		try
		{
		sleep((long) Math.random()*1000);
		}
		catch (InterruptedException e){}
		
	}
	
	public void wjazd()
	{
		sumaWag+=waga;
		System.out.println(sumaWag);
		try
		{
			sleep(1000);
		
		}catch (InterruptedException e){e.printStackTrace();}

		sumaWag-=waga;
	}
	
	public void run()
	{
		for (int i =0; i<20; i++)
		{
			zjazd();
			wjazd();
		}
	}
}

Po 1) Po kiego grzyba dziedziczyć po wątku zamiast implementowac Runnable?
Po 2) Generalnie te współdzielenie zmiennych w ten sposób jest słabe, powinien być synchronizowany dostęp
Myślę że powinień jeszcze raz przejrzeć podstawy Javy

o_O Dzieją sie cuda bo masz tu race-condition wszędzie. Jakbyś chciaż miał tam jakiś AtomicInteger na tych licznikach, to miało by to ręce i nogi, a tak to może się dziać cokolwiek bo masz tu zero memory barrier, więc brak synchronizacji pamięci pomiędzy wątkami (no ten printf akurat trochę tu pomaga co najwyżej, ale to przypadkiem...) i brak kontroli atomicznych zmian wartości.

Pomyśl np. co to jest
sumaWag+=waga;
To jest operacja:

1. Załaduj aktualną wartość zmiennej sumaWag to rejestru procesora
2. Załaduj aktualną wartość zmiennej waga do rejestru procesora
3. Zsumuj te dwie wartości
4. Zapisz do pamięci pod zmienną sumaWag wynik sumowania

I u ciebie każda z tych operacji może być przeplatana operacjami z innych wątków! Więc jeśli np. dwa wątki robią +1 ale drugi wątek wystartuje zanim pierwszy wątek zapisze swój wynik, to oba wątki jako "aktualną" wartość wczytają tą samą liczbę i potem drugi wątek zapisze swój wynik, nadpisując wynik wątku pierwszego. W efekcie wynikiem będzie x+1 a nie oczekiwane x+2...

Witam,

masz tu jedną sekcję krytyczną, z którą musisz sobie poradzić.

    private static int sumaWag=0;

Jak?

• lock na tą zmienną
• słówkiem kluczowym volatile
• może refaktoryzacją

Wszystko to co poprzednicy, plus jakieś podstawy poczytać:
http://edu.pjwstk.edu.pl/wyklady/zap/scb/W9/W9.htm

Dziękuję wszystkim za uwagi. Były pomocne. Dążyłem do zrobienia zadania o poniższej treści. Jestem ciekaw czy wygląda to już lepiej? Pojawiły się też nowe problemy.

Wyciąg narciarski o pewnej ładowności N
Klienci posiadają ciężar (losowa wartość Ki) i są wątkami wykonującymi w
pętli:

1. zjazd (sleep(duża losowa wartość))
2. próbują wjechać wyciągiem (jeśli suma ciężaru wciąganych klientów jest
   mniejszą bądź równa N),
3. jeśli nie udaje się to czekają (sleep(niewielka losowa wartość) i
   ponownie wykonują poprzedni punkt
4. jeśli się udaje dostać to wjeżdżają wyciągiem (sleep(stała wartość dla
   wszystkich klientów))

Oto efekt:

public class Klient extends Thread 
{
	private Wyciąg wyciąg;
	private int waga;
	private int id=0;
	private static int NextId=1;

	
	public Klient(Wyciąg awyciąg, int w)
	{
		
		this.wyciąg = awyciąg;
		this.waga=w;
	}
	
	public int getWaga()
	{
		return waga;
	}

	public void run()
	{
		for (int i =0; i<10; i++)
		{
			wyciąg.zjazd(this);
			wyciąg.wejście(this);
			wyciąg.wjazd(this);
			wyciąg.zejscie(this);
		}
	}
}

public class Wyciąg 
{
	
	private static int zaladowanie=0;
	private static final int max_zaladowanie=60; 
	
	synchronized public void wejście(Klient klient)
	{
		try
		{
			System.out.println("Klient " + klient.getName() + " Próbuję wejść na wyciąg");
			while (zaladowanie>max_zaladowanie)
			{
				System.out.println("Wyciąg pełny - czekaj!");
				klient.sleep((long) Math.random()*1000);
				wait();
			}
		}
		catch (InterruptedException e) {}
		System.out.println("Klient " + klient.getId() + "wszedl na wyciąg");
		zaladowanie+=klient.getWaga();
		System.out.println("Zaladowanie = " + zaladowanie);
		
	}
	
	synchronized public void zejscie (Klient klient)
	{

		System.out.println("Klient "+ klient.getId() + " Schodzi z wyciągu");
		zaladowanie-=klient.getWaga();
		System.out.println("Aktualne zaladowanie = " + zaladowanie);
		notify();
	}
	
	public void zjazd(Klient klient)
	{
		
		System.out.println("Klient nr: " + klient.getId()+ "Zjezdza z górki");
		try
		{
		klient.sleep((long) Math.random()*5000);
		}
		catch (InterruptedException e){}
		
	}

	public void wjazd(Klient klient)
	{
		
		try
		{
			
		klient.sleep(10000);
		
		}
		catch (InterruptedException e){e.printStackTrace();}
	}

}

Zadanie jeszcze nie jest skończone ponieważ trafiłem na kolejne problemy:

1. Jeśli wyciąg jest przeciążony i kilka wątków czeka na wejście, Jak zrobić tak by pierwszy wszedł ten, który pierwszy podjął taką próbę? (Jak w kolejce, kto pierwszy się ustawił, pierwszy wchodzi)
2. Nie potrafię spełnić 3 punktu zadania

iii. jeśli nie udaje się to czekają (sleep(niewielka losowa wartość) i ponownie wykonują poprzedni punkt
W moim przypadku jest tak, że wątek podejmuje tylko jedną nieudaną próbę i nie podejmuje kolejnych, tylko czeka, aż inny wątek opuści wyciąg.

1. Nie do końca potrafię zrozumieć koncepcji monitora. Co jest u mnie monitorem , o ile w ogóle jest?

Z góry dziękuję :)

ad 1. Kolejka to taka sprytna struktura danych, która pozwala na zachowanie kolejności. W Javie od biedy może być lista.
ad 2. Ponieważ robisz pojedyncze sleep, które blokuje wątek, ale on nadal działa. Poczytaj o wait.
ad 3. Monitor to obiekt, który pełni rolę "strażnika" dla bloku synchronizowanego. W twoim przypadku, jako że używasz synchronize na metodzie, to monitorem jest obiekt, na którym metodę wywołujesz.