0

I have thread with Socket inside. When soket receives some data, I need to fire event (using my sublassed EventObject). Event listeners is added from main thread (to some list?). is this OK?

Pseudocode:



public class SocketThread extends Thread{
    private Socket socket;
    private MyEventListener eventListener;

    public SocketThread(Socket socket, MyEventListener eventListener) {
        this.socket=socket;
        this.eventListener=eventListener;
    }

   public void run() {
        get socket input stream...
        get socket output stream...
        when data received, call process(data) 
    }

   void process(data){
     synchronized(this){
        myEvent event=new MyEvent(data);
        eventListener.fireSomeEvent(myEvent );
    }
}


// main thread

ServerSocket serverSocket=new ServerSocket(host,port);
Socket socket= serverSocket.accept();
ClientThread cthr = new SocketThread (sckt,new MyEventListener(){ 
    void fireSomeEvent(MyEvent event){
    //some code
    }
});
Sanyin
  • 41
  • 2
  • 8
  • I think your solution is generally ok, but I'm thinking you might benefit from implementing the [producer-consumer pattern](http://java.dzone.com/articles/producer-consumer-pattern). Your socket thread produces messages which are added to a blocking queue, which the other class (the event listener you have presently) grabs from whenever a new message / data is available. – Paul Richter Feb 28 '14 at 16:07
  • Should I remove syncronization block? – Sanyin Feb 28 '14 at 16:16
  • Yes, if you do the producer-consumer pattern and you use a blocking queue, you don't need the synchronization block as [the blocking queue's add / take methods are thread safe](http://stackoverflow.com/a/2695437/877472). – Paul Richter Feb 28 '14 at 17:15

1 Answers1

0

This seems perfectly fine. As long as you're not changing the eventListener there is no concurrency problem that way. But be aware that fireSomeEvent() will be ran from the SocketThread. If you're doing something that is not thread-safe there, you might (will) encounter problems. So that's where you'll need some sort of synchronization.

ddmps
  • 4,350
  • 1
  • 19
  • 34
  • I know events will be fired from the SocketThread. If fired Event is accessing some main thread's data, how to lock that data from fired event? I am firing that event from synchronized block already (should I remove that?) – Sanyin Feb 28 '14 at 16:14
  • That synchronized block is only good for not firing several events concurrently from one socket (which shouldn't happen anyway if your run method fires them without any new threads). I think what you are looking for is synchronizing on the main object (note: object, not thread). – ddmps Feb 28 '14 at 16:44
  • But it is good for firing several events, using some list, which are added in the same thread (main), inside for loop? – Sanyin Feb 28 '14 at 17:07