0

In my application I keep track of the running threads via a hash map, in that way I can retrieve the correct thread and do any actions. The app evolved in a way that new threads are created and old threads must be destroyed. My concern is if I'm removing the elements of the hash correctly, or if there's any efficient pattern.

I'm developing in Java but I'll share the pseudo code:

# HashMap mainHashmap already initialized
List toDelete = new List();
# First step, stop the threads
For (each element in HashMap)
{
  if (element.value is ThreadA)
  {
    element.value.stopThread();
    toDelete.add(element.key);
  }
 }
 # Then remove the element from the array
 For (each key in toDelete)
 {
    mainHashmap.remove(key);
 }

Thanks

5
  • Most hash implementations provide a remove() method, which is the way to remove mappings. Are you looking for something beyond that? (And why are you building a separate list of things to delete when you could iterate over the hash and remove each mapping when you stop each thread?)
    – Blrfl
    Commented May 2, 2013 at 10:43
  • @Blrfl: the separate list is needed because if one calls "remove" within the first loop, the iterator of the loop gets invalidated. This is the standard approach to deal with that kind of problem.
    – Doc Brown
    Commented May 2, 2013 at 11:00
  • I'm using this method, I'm just looking for the correct way to remove elements from an array that contains threads. And as I see from Neil's answer I start to see what's the problem. Before I didn't have to remove them, they were running for the time the app was up. Now I need to remove previous ones being sure they are stopped.
    – user50236
    Commented May 2, 2013 at 11:01
  • @DocBrown: Not if you're using an an Iterator, which provides a safe way to remove the current item. Should've made that clearer in my comment. The code as-is wouldn't be able to work on an arbitrarily-large map.
    – Blrfl
    Commented May 2, 2013 at 13:58
  • @Blrfl You're right, and I use often the Iterators in Java, but in this case I'm looking a general solution based on pseudo code. I'm not concern about what each programming language does, that would lead to another topic (in my opinion)
    – user50236
    Commented May 2, 2013 at 14:03

1 Answer 1

2

A couple problems I can see with this are the following:

  1. Stopping a thread like this is dangerous since you have no idea what it is doing in that particular moment. Usually in a situation like this you'd send it a message asking it to stop when it's convenient, and the thread can stop in its own way (closing resources and connections, etc.). Once each thread is stopped (when the callback indicating that it has stopped is called), then you can take the proper action of removing it from your hash map.

  2. The second problem is a little more subtle. I see no locks here, so I have to assume there are none. You have to be sure that this method cannot be run while another thread is running it or you get all sorts of problems which seem to happen only once in a while in random moments. In general, any access to this hash map should be locked or should not be public and the methods you use to access it are locked.

If the threads being run are your own, and you're fairly confident that these threads can be stopped on a dime, then #1 is not a problem, however know that it is an anti-pattern. Most languages support stopping a thread abruptly only because it should still exist as a last resort option, but you would normally ask nicely if the thread could please stop for you. If the thread ignores the request (especially these threads are not running code you've made yourself), you can then consider the possibility of outright stopping the thread.

I can't get too specific because it really depends on the language that you're using, but most languages have a proper "done" method which gets executed upon finish (or halt), and you could take advantage of this method by calling the thread pool with that instance to indicate that the thread is stopped and can be removed.

3
  • Nice answer, about the 1st point, I implemented the stop that simply get while value to false and nicely stop by itself. The second part, I didn't think about it, I'll investigate further.
    – user50236
    Commented May 2, 2013 at 11:06
  • #2 is only a problem if there will be more than 1 thread accessing HashMap.
    – Doc Brown
    Commented May 2, 2013 at 11:06
  • @DocBrown You're right about #2, however you can never be too careful. If you're making a web application for a shared thread pool, it's already something that needs to be remedied either inside the class or outside. He either needs to be aware that it is not thread-safe or he needs to make it thread-safe in any case in my humble opinion.
    – Neil
    Commented May 2, 2013 at 12:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.