Bug 315923

Comment 1 Randy Roberts 2010-06-07 12:23:42 EDT

Is this reliably reproducible?

I can try to fix this, but with ConcurentModificationException's it
can be hit or miss.

Comment 2 Randy Roberts 2010-06-07 12:26:12 EDT

(In reply to comment #1)
> Is this reliably reproducible?
> 
> I can try to fix this, but with ConcurentModificationException's it
> can be hit or miss.

I cannot reproduce with OpenMPI RM.

Comment 3 Greg Watson 2010-06-07 12:47:41 EDT

I only saw it once.

Comment 4 Greg Watson 2010-06-07 12:51:03 EDT

The only way I can see this occurring is if getAttributeIndexSet is called during removeProcessesByJobRanks

Comment 5 Greg Watson 2010-06-07 12:52:04 EDT

So you probably need to synchronize processAttributesMap

Comment 6 Randy Roberts 2010-06-07 12:55:25 EDT

(In reply to comment #5)
> So you probably need to synchronize processAttributesMap

I've synchronized the following PJob methods
  getAttributeIndexSet(D)
  getProcessAttributeKeys(BitSet)
  getProcessAttributes(BitSet)
  getProcessAttributes(String, BitSet)
  removeProcessesByJobRanks(BitSet)

I've committed these.

Comment 7 Greg Watson 2010-06-07 13:00:55 EDT

Thanks. Let's hope that fixes the problem. I'll reopen if I see the problem again.

Comment 8 Greg Watson 2010-08-19 15:25:45 EDT

Unfortunately this now causes a deadlock in the model when the debugger launches. Currently launching more than a one process debug job deadlocks the UI in HEAD.

The problem is that the resource manager calls PJob#addProcessesByJobRanks to add new processes to the job. This in turn calls PJob#fireNewProcesses to indicate the new processes have been created. The debug view has registered a handler on the job to get notified of process changes and is called when this happens. The handler in turn calls getAttribute() to find the status of the process (in order to display the correct icon). This calls PProcess#getAttribute which calls PJob#getProcessAttribute. Since both PJob#addProcessesByJobRanks and PJob#getProcessAttribute are synchronized, we get a deadlock.

The synchronization used on PJob (and the other model element classes) needs to be checked to ensure that this kind of deadlock can't occur. I'd say you need to use finer grain synchronization than the whole method, particularly on methods that invoke handlers.

Comment 9 Randy Roberts 2010-08-19 15:31:26 EDT

(In reply to comment #8)

Do you think I can just exclude the firing of the handlers
from the synchronization?

> Unfortunately this now causes a deadlock in the model when the debugger
> launches. Currently launching more than a one process debug job deadlocks the
> UI in HEAD.
> 
> The problem is that the resource manager calls PJob#addProcessesByJobRanks to
> add new processes to the job. This in turn calls PJob#fireNewProcesses to
> indicate the new processes have been created. The debug view has registered a
> handler on the job to get notified of process changes and is called when this
> happens. The handler in turn calls getAttribute() to find the status of the
> process (in order to display the correct icon). This calls
> PProcess#getAttribute which calls PJob#getProcessAttribute. Since both
> PJob#addProcessesByJobRanks and PJob#getProcessAttribute are synchronized, we
> get a deadlock.
> 
> The synchronization used on PJob (and the other model element classes) needs to
> be checked to ensure that this kind of deadlock can't occur. I'd say you need
> to use finer grain synchronization than the whole method, particularly on
> methods that invoke handlers.

Comment 10 Randy Roberts 2010-08-19 15:40:57 EDT

(In reply to comment #9)
If so I will just unsynchronize
  PJob#addProcessAttributes(BitSet processIds, AttributeManager attributes)
  PJob#removeProcessesByJobRanks(BitSet processJobRanks)
and
  PJob#addProcessesByJobRanks(BitSet newProcessJobRanks, AttributeManager attrs)

and do the main stuff in a 

   synchronized (this) {
      block
   }

followed by an unsynchronized call
to the respective fireXXXProcesses(...) call.

> (In reply to comment #8)
> 
> Do you think I can just exclude the firing of the handlers
> from the synchronization?
>

Comment 11 Greg Watson 2010-08-19 16:12:12 EDT

I would have thought that you'd only want to synchronize access to the data structures, so calling the handlers outside the synchronization is definitely a good idea. It seems to have fixed the problem anyway.

Comment 12 Greg Watson 2012-10-26 09:59:19 EDT

Superceded by new RM framework.