Bug 199573

Summary: Threading issues in SystemTempFileListener
Product: [Tools] Target Management Reporter: Martin Oberhuber <mober.at+eclipse>
Component: RSEAssignee: Martin Oberhuber <mober.at+eclipse>
Status: RESOLVED FIXED QA Contact: Martin Oberhuber <mober.at+eclipse>
Severity: minor    
Priority: P2 CC: javier.montalvoorus, kjdoyle, xuanchen
Version: 2.0Flags: javier.montalvoorus: review+
Target Milestone: 2.0.1   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Patch to fix threading issues none

Description Martin Oberhuber CLA 2007-08-10 10:09:24 EDT 
There are threading issues in SystemTempFileListener:

* The _isSynching variable is not synchronized between main thread and worker
  thread, it should be marked volatile

* The _changedResources ArrayList is modified in the main thread (by clearing
  it), but at the same time another thread may call add(), this may lead to
  a ConcurrentModificationException

* In case an exception happens in the RefreshResourcesUIJob, the _isSynching
  variable is not reset so it can never sync again
Comment 1 Martin Oberhuber CLA 2007-08-10 10:13:52 EDT 
Created attachment 75845 [details]
Patch to fix threading issues

Attached patch fixes the issues:

* Declare _isSynching volatile to fix multi-thread access

* Make the synchronized(_changedResources) block smaller, by first copying
  contents into a separate array then clearing the arrayList; that way, we
  are sure to never call out of the synchronized block, avoiding deadlock.

* Add synchronzied(_changedResources) around the _changedResources.add()
  call in order to avoid ConcurrentModificationException

* Put try ... finally block around the code between the _isSynching mutex
Comment 2 Martin Oberhuber CLA 2007-08-10 10:15:00 EDT 
Patch committed:
[199573] Fix potential threading issues in SystemTempFileListener
   SystemTempFileListener.java  1.21

Everybody please review the changes for educational purposes. It's a very typical example of how threading issues are to be avoided in Java.