Threading - schreiben auf Hashmap/löschen - ConcurrentModificationException

[Kyaha]

Hallo alle miteinander!

Ich versuche derzeit eine Hashmap durch 2 Threads zu verwalten.
Dabei soll der erste Thread Daten reinschreiben und der zweite Thread soll sich dann in Intervallen die Daten überprüfen und unnütze Daten rauswerfen.

Dafür wollte ich eine HashMap<String,Long> die einen String einer Milisekundenanzahl zuordnet - sozusagen der "letzte Zugriff".

Nur leider bekomme ich dies leider nicht ohne eine ConcurrentModificationException umgesetzt. Obwohl diese fliegt wird in meiner Hashmap korrekt der Eintrag entfernt, aber ich finde den Fehler einfach nicht.

Anbei der Code meiner zwei Klassen:

package my.ThreadTest2;

import java.util.HashMap;
import java.util.Hashtable;
import java.util.Iterator;

public class Go extends Thread {

	public static HashMap<String,Long> hmap;
	
	public static void main(String[] args) {
			
		hmap = new HashMap<String,Long>();
		long time = System.currentTimeMillis();
		hmap.put("User1", time);
		hmap.put("User2", time);
		hmap.put("User3", time);
		
		Deleter d = new Deleter(hmap);
		d.start();
		

		System.out.println("1st sleep - 40 sec");
		
		try {
			sleep(1000*40);
		} catch (InterruptedException e) {
			e.printStackTrace();
		}
		System.out.println("waking up!");
		System.out.println("updating user1 and user 3");

		time = System.currentTimeMillis();
		
		synchronized(hmap) {
			hmap.put("User1",time);
			hmap.put("User3", time);
		}//synchronized
		Iterator<String> it = hmap.keySet().iterator();
		
		String key;
		
		while (it.hasNext()) {
			key = it.next();
			System.out.println("user: " + key + " with last access at " + hmap.get(key));
		}//while
		
		
		System.out.println("2nd sleep - 40 sec");
		
		try {
			sleep(1000*40);
		} catch (InterruptedException e) {
			e.printStackTrace();
		}
		System.out.println("waking up!");
		
		it = hmap.keySet().iterator();
		while (it.hasNext()) {
			key = it.next();
			System.out.println("user: " + key + " with last access at " + hmap.get(key));
		}//while
		
		System.out.println("updating User3");
		synchronized(hmap) {
			hmap.put("User3", System.currentTimeMillis());
		}//synch
		
		System.out.println("3rd sleep - 60 sec");
		
		try {
			sleep(1000*60);
		} catch (InterruptedException e) {
			e.printStackTrace();
		}
		System.out.println("Waking up!");
		
		System.out.println("now last should be shown as deleted!");
		
		it = hmap.keySet().iterator();
		while (it.hasNext()) {
			key = it.next();
			System.out.println("user: " + key + " with last access at " + hmap.get(key));
		}//while
		
	}//main
	
	
}//class

package my.ThreadTest2;

import java.util.HashMap;
import java.util.Hashtable;
import java.util.Iterator;

public class Deleter extends Thread {

	private HashMap<String,Long> h;
	
	public Deleter (HashMap<String,Long> hh) {
		h = hh;
	}//constructor
	
	public void run() {
		
		int sec = 60;
		long time;
		
		String key;
		
		int delay = 1000*30;
		
		while (true) {
			
			try {
				sleep(1000*sec);
			} catch (InterruptedException e) {
				e.printStackTrace();
			}//try catch
			
			time = System.currentTimeMillis();
			synchronized(h) {
			//over the hashmap
			Iterator<String> it = h.keySet().iterator();
			while(it.hasNext()) {
				key = (String) it.next();
				if (h.get(key) < time-delay) {
					System.out.println("user with key " + key + "deleted!\ncurrent time milis " + time + "\nmax delay " + delay);
					System.out.println("last access vom user " + h.get(key));
					
					h.remove(key);
					
				}//if
				
			}//while
			}//synch
			
		}//while
		
		
	}//run
	
}//class

Kann man dies überhaupt so realisieren? Wenn ja, ist der Ansatz soweit korrekt oder befinde ich mich auf dem Holzweg? Wie kommt es zu dieser Exception obwohl doch alles korrekt durchgearbeitet wird?

Vielen Dank für die Mühen im voraus!

Beste Grüße

Kyaha

 

[Marco13]

Ohne den Code jetzt im Detail gelesen zu haben: Wenn man mit zwei Threads auf eine Collection zugrift, muss man sicherstellen, dass die Collection synchronized ist.

Im einfachsten Fall geht das, indem man die Zeile
private Map<X,Y> map = new HashMap<X,Y>();
ändert in
private Map<X,Y> map = Collections.synchronizedMap(new HashMap<X,Y>());

Falls du irgendwo mit einer for-Schleife über die Collection iterierst, muss die noch explizit darauf synchronized werden:

synchronized(map)
{
    for (X x : map.keySet()) { .... }
}

 

[KSG9|sebastian]

Problem ist dass du nen Iterator benutzt und somit die Liste für Änderungen "sperrst".

 

[Ariol]

package my.ThreadTest2;

import java.util.Iterator;
import java.util.concurrent.ConcurrentHashMap;

import org.omg.PortableInterceptor.USER_EXCEPTION;

public class Deleter extends Thread
{

	private ConcurrentHashMap<String, Long>	map;

	public Deleter(ConcurrentHashMap<String, Long> hh)
	{
		map = hh;
	}// constructor

	public void run()
	{

		int minute = 60 * 1000;;
		int delay = minute/2;

		while (true)
		{

			try
			{
				sleep(minute);
			}
			catch (InterruptedException e)
			{
				e.printStackTrace();
			}// try catch

			long time = System.currentTimeMillis();
			// over the hashmap
			Iterator<String> it = map.keySet().iterator();
			while (it.hasNext())
			{
				String key = (String) it.next();
				if (map.get(key) < time - delay)
				{
					System.out.println("user with key " + key + "deleted!\ncurrent time milis " + time + "\nmax delay " + delay);
					System.out.println("last access vom user " + map.get(key));

					map.remove(key);

				}// if
			}// while
		}// while
	}// run
}// class

package my.ThreadTest2;

import java.util.Iterator;
import java.util.concurrent.ConcurrentHashMap;

public class Go extends Thread
{

	public static ConcurrentHashMap<String, Long>	map;

	public static void main(String[] args)
	{

		long time = System.currentTimeMillis();

		map = new ConcurrentHashMap<String, Long>();
		map.put("User1", time);
		map.put("User2", time);
		map.put("User3", time);

		Deleter d = new Deleter(map);
		d.start();

		System.out.println("1st sleep - 40 sec");

		try
		{
			sleep(1000 * 40);
		}
		catch (InterruptedException e)
		{
			e.printStackTrace();
		}
		
		System.out.println("waking up!");
		System.out.println("updating user1 and user 3");

		time = System.currentTimeMillis();
		map.put("User1", time);
		map.put("User3", time);
		
		Iterator<String> it = map.keySet().iterator();
		String key;

		while (it.hasNext())
		{
			key = it.next();
			System.out.println("user: " + key + " with last access at " + map.get(key));
		}// while

		
		System.out.println("2nd sleep - 40 sec");
		try
		{
			sleep(1000 * 40);
		}
		catch (InterruptedException e)
		{
			e.printStackTrace();
		}
		System.out.println("waking up!");

		it = map.keySet().iterator();
		while (it.hasNext())
		{
			key = it.next();
			System.out.println("user: " + key + " with last access at " + map.get(key));
		}// while

		System.out.println("updating User3");
		map.put("User3", System.currentTimeMillis());
		
		System.out.println("3rd sleep - 60 sec");
		try
		{
			sleep(1000 * 60);
		}
		catch (InterruptedException e)
		{
			e.printStackTrace();
		}
		System.out.println("Waking up!");

		System.out.println("now last should be shown as deleted!");

		it = map.keySet().iterator();
		while (it.hasNext())
		{
			key = it.next();
			System.out.println("user: " + key + " with last access at " + map.get(key));
		}// while

	}// main

}// class

Mit der richtigen Map brauchst du kein synchronized :wink: