2

I have a singleton class that adds and removes clients (applets) to/from a linked list like so:

public class ClientManager {
    //Collections.unmodifiableList
    private static ClientManager instance = new ClientManager();
    private static LinkedList<Bot> Clients = new LinkedList<>();

    //private constructor here..

    public static Bot add(final Bot bot) {
        Clients.add(bot);
        new Thread(new Runnable() {
            @Override
            public void run() {
                while (bot.getApplet() == null) {
                    Utilities.sleep(10);
                }
            }

        }).start();
        return bot;
    }

    public static Bot remove(int hashCode) {
        for (Iterator<Bot> it = Clients.iterator(); it.hasNext();) {
            Bot bot = it.next();
            if (bot.getCanvas().getClass().getClassLoader().hashCode() == hashCode) {
                it.remove();
                return bot;
            }
        }
        return null;
    }

    public static Bot getBot(int hashCode) {
        for (Bot bot : Clients) {
            if (bot.getCanvas() != null && (bot.getCanvas().getClass().getClassLoader().hashCode() == hashCode)) {
                return bot;
            }
        }
        return null;
    }
}

Then I have a JFrame with a JTabbedPane which has an exit button on each Tab with the actionListener:

CloseButton.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                Frame.this.removeBot((Loader) component);
            }
        });

//Need to synchronize this function some how so that when I run it in a new Thread, they don't all access the LinkedList at the same time..

private void removeBot(final Loader loader) {
        Frame.this.TabPanel.remove(loader);   //Need to synchronize this or tabbedpane throws.. This call removes a tab..

        List<Bot> Clients = ClientManager.getBots();

        for (Iterator<Bot> it = Clients.iterator(); it.hasNext();) {
            Bot bot = it.next();
            if (bot != null && bot.getLoader() != null && bot.getLoader() == loader) {
                it.remove();
                loader.destruct(); //Destroy my applet. Calls applet.stop() and then applet.destroy()
            }
        }
        System.gc();
    }

Question:

How I can synchronize the removal of Tabs so that the TabbedPane doesn't throw when multiple threads tries to remove a tab and how to synchronize my removal function so that multiple threads don't delete from the LinkedList at the same time because its throwing errors if I run any of the above in a new Thread..

I tried looking at the tutorials and I put synchronized before my functions but that doesn't help.

Brandon
  • 22,723
  • 11
  • 93
  • 186
  • 1
    +1 for [sscce](http://sscce.org) – MarioDS May 06 '13 at 18:55
  • [I Shall Call It.. SomethingManager](http://www.codinghorror.com/blog/2006/03/i-shall-call-it-somethingmanager.html). Also, in regards to your first sentence "I have a singleton". I don't know if you've read much about them, but [here's a good discussion about them](http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons). – Patrick May 06 '13 at 19:32
  • I have to use a singleton because how else would I have all classes access a single resource? I want the resource to be accessible by all classes and methods and singleton fits that description as it tracks how many bots I have running.. And ok, I'll rename the manager to ClientPool. – Brandon May 06 '13 at 20:07

1 Answers1

1

Add 2 locks:

a)

    synchronized( Frame.this.TabPanel ) {
       ... remove etc until the end of the method
    }

Add this also to the code where you add something to TabPanel:

    synchronized( Frame.this.TabPanel ) {
       ...add the loader here...
    }

b)

The same for the list of the bots:

    synchronized( Clients ) {
       Clients.add(bot);
        new Thread(new Runnable() {
        @Override

        i.e. the contents of the add method
    }

The same at the beginning of the remove method

public static Bot remove(int hashCode) {
   synchronized( Clients ) {
    for (Iterator<Bot> it = Clients.iterator(); it.hasNext();) {
        Bot bot = it.next();
        if (bot.getCanvas().getClass().getClassLoader().hashCode() == hashCode) {
            it.remove();
            return bot;
        }
    }
    return null;
  }// synchronized
}

And then in removeBot!

    synchronized( Clients ) {
    for (Iterator<Bot> it = Clients.iterator(); it.hasNext();) {
        Bot bot = it.next();
        if (bot != null && bot.getLoader() != null && bot.getLoader() == loader) {
            it.remove();
            loader.destruct(); //Destroy my applet. Calls applet.stop() and then applet.destroy()
        }
    }
    }
xtraclass
  • 445
  • 3
  • 7
  • 1
    Ohh wow.. I added two variables for locking the TabbedPane since it isn't final I had to do: private static final Object TabPaneLock = new Object(); And synchronized(TabPaneLock); Can I replace the synchronized(Frame.this.TabbedPane) with the Object Lock? – Brandon May 06 '13 at 19:17
  • 1
    Sure, you can replace sync...( TabbedPane ) with sync...( TabPaneLock ) - it's actually better to use dedicated lock objects. – xtraclass May 07 '13 at 17:35