Search code examples
javamultithreadingsocketsvolatile

Java Concurrent Sockets: not being able to share a variable between threads


I happen to have a problem with some attempts at reading the same variable in socket multi-threading, not being able to share it among threads.

It works as an app where an employer assigns work to an employee. Through his interface the employer can add and assignment to an ArrayList inside a class named ListadoPedidos.

When the employer's ServerSocket accepts an employee Socket, it starts a TCP connection and launches the following thread:

public class HiloServer implements Runnable{

	private ListadoPedidos peds=new ListadoPedidos();
	private ListadoOperarios operarios=new ListadoOperarios();
	private ListadoSockets sockets=new ListadoSockets();
	private SocketServer s;
	
	public HiloServer(SocketServer sock, JFrame frame, ListadoPedidos pedidos) {
		s=sock;
		peds=pedidos;
	}

	/* (non-Javadoc)
	 * @see java.lang.Runnable#run()
	 */
	@Override
	public void run() {
		boolean agregar;
		Socket nuevo;
		try {
			while(true) {
				// ACEPTA OPERARIOS QUE DESEEN CONECTARSE
				s.aceptar();
				nuevo=s.getSocket();
				sockets.addSocket(nuevo);
				new NuevoCliente();
				HiloDatos hd=new HiloDatos(s, nuevo,operarios,peds,sockets);
				Thread t=new Thread(hd);
				t.start();
			}
		}
	   catch (IOException e) {
			e.printStackTrace();
		}
		
	}
}

*note that I send the object where the assigments added are stored.

Then it starts another thread that will work as a sort of "validation" for a number the employee's have to insert and send through his Swing interface to truly enter the system. This thread is generated everytime a new socket employee makes a TCP connection to the ServerSocket employer. It goes like:

public class HiloDatos implements Runnable {

	private int n;
	private Socket cliente;
	private SocketServer server;
	private int opRecibido;
	private ListadoOperarios ops;
	private ListadoPedidos peds;
	private ListadoSockets socks;
	
	public HiloDatos(SocketServer ss, Socket nuevo, ListadoOperarios operarios, ListadoPedidos pedidos, ListadoSockets sockets) {
		cliente=nuevo;
		server=ss;
		ops=operarios;
		peds=pedidos;
		socks=sockets;
	}

	@Override
	public void run() {
		server.setSocket(cliente);
		boolean agregar, aceptado=false;
		try {
			do {
				// RECIBE EL NRO OPERARIO Y VERIFICA SU EXISTENCIA
				agregar=true;
				opRecibido=Integer.parseInt(server.recibir()); 
				for(int c=0;c<ops.getOperarios().size();c++) {
					if (opRecibido==ops.getOperarios().get(c)) {
						new ErrorRepetido();
						agregar=false;break;
					}
				}
				if (agregar==true) {
					ops.addOperarios(opRecibido);
					server.enviar("Si");
					aceptado=true;
				}
				}while(aceptado==false);
				HiloPedidos hp=new HiloPedidos(server,opRecibido,ops,peds,socks);
				Thread t=new Thread(hp);
				t.start();
	
				}catch (NumberFormatException e) {
					new ErrorDatos();
				} catch (ConnectException e) {
					new ErrorConexion();
				} catch (SocketException e) {
					try {
						socks.getSockets().remove(socks.getSockets().indexOf(cliente));
						cliente.close();
					} catch (IOException e1) {
						new ErrorFlujo();
					}
					new WarnSocket();
				} catch (IOException e) {
					try {
						socks.getSockets().remove(socks.getSockets().indexOf(cliente));
						cliente.close();
					} catch (IOException e1) {
						new ErrorFlujo();
					}
					new WarnFlujo();
				}
			}
		}

And lastly it launches yet another Thread that looks for that same validation number from the thread above in the ArrayList of assignments ("pedidos" of class ListadoPedidos) i kept passing from thread to thread, and if it finds a "new" one, it should send it to the connected socket:

public class HiloPedidos implements Runnable {

	private Pedido ped;
	private SocketServer server;
	private int op;
	private ListadoOperarios ops;
	private ListadoPedidos peds;
	private ListadoSockets socks;
	
	public HiloPedidos(SocketServer ss, int opRecibido, ListadoOperarios operarios, ListadoPedidos pedidos, ListadoSockets sockets) {
		server=ss;
		opRecibido=op;	
		ops=operarios;
		peds=pedidos;
		socks=sockets;
	}

	@Override
	public void run() {
		int cambio=0, nuevo;
		Pedido pedRecibido;
		try {
			while(true) {
				// ENVÍA PEDIDOS
				nuevo=peds.Contar(op);
				if(nuevo==cambio) {
					cambio=peds.Contar(op);
					pedRecibido=peds.TraerNuevo(op, cambio);
					server.enviarObjeto(pedRecibido);
				}
			}}
				catch (NumberFormatException e) {
					new ErrorDatos();
				} catch (ConnectException e) {
					new ErrorConexion();
				} catch (SocketException e) {
					try {
						socks.getSockets().remove(socks.getSockets().indexOf(server.getSocket()));
						server.getSocket().close();
					} catch (IOException e1) {
						new ErrorFlujo();
					}
					new WarnSocket();
				} catch (IOException e) {
					try {
						socks.getSockets().remove(socks.getSockets().indexOf(server.getSocket()));
						server.getSocket().close();
					} catch (IOException e1) {
						new ErrorFlujo();
					}
					new WarnFlujo();
				}
			}
		}

Problem is that the last thread can't really notice a change in the list, as i debugged it and never reached the breakpoint inside the condition of sending the assignment. The class ListadoPedidos goes like this:

public class ListadoPedidos {
	
	private static volatile ArrayList<Pedido> pedidos=new ArrayList<>();
	
	public ListadoPedidos() {
		
	}

	public ArrayList<Pedido> getPedidos() {
		return pedidos;
	}

	public synchronized void addPedidos(Pedido pedido) {
		pedidos.add(pedido);
	}
	
	public int Contar(int o) {
		int n=0;
		for (Pedido p: pedidos) {
			if (p.getNro_operario()==o) {
				n++;
			}
		}
		return n;
	}
	
	
	public Pedido TraerNuevo(int o, int c) {
		int n=0;
		Pedido nuevo = new Pedido();
		for (Pedido p: pedidos) {
			if (p.getNro_operario()==o) {
				n++;
			}
			if (n==c) {
				nuevo=p;break;
			}
		}
		return nuevo;
	}
	
	
}

Contar is the one that counts for an assignment with the value nrooperario same as the value it brings from the thread, and TraerNuevo brings the assignment to be sended (never reached this method).

I tried declaring the ArrayList as volatile and all but nothing works. Mind that even if i use socket connections, the problem has more to do with shared varaible not being able to update between threads. Any help will be appreciated.


Solution

  • Try this, basically, synchronize access.

    public class ListadoPedidos {
    
    private static volatile ArrayList<Pedido> pedidos=new ArrayList<>();
    
    public ListadoPedidos() {
    
    }
    /**
     * Here DO NOT return the arrayList. The underlying implementation is not threadsafe
     */
    //    public  ArrayList<Pedido> getPedidos() {
    //        return pedidos;
    //    }
    
    public synchronized void addPedidos(Pedido pedido) {
        pedidos.add(pedido);
    }
    
    public synchronized int Contar(int o) {
        int n=0;
        for (Pedido p: pedidos) {
            if (p.getNro_operario()==o) {
                n++;
            }
        }
        return n;
    }
    
    
    public synchronized Pedido TraerNuevo(int o, int c) {
        int n=0;
        Pedido nuevo = new Pedido();
        for (Pedido p: pedidos) {
            if (p.getNro_operario()==o) {
                n++;
            }
            if (n==c) {
                nuevo=p;break;
            }
        }
        return nuevo;
    }
    
    }