Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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

share|improve this question
    
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 May 2 '13 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 May 2 '13 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. –  jmolinaso May 2 '13 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 May 2 '13 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) –  jmolinaso May 2 '13 at 14:03

1 Answer 1

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.

share|improve this answer
    
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. –  jmolinaso May 2 '13 at 11:06
    
#2 is only a problem if there will be more than 1 thread accessing HashMap. –  Doc Brown May 2 '13 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 May 2 '13 at 12:04

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.