It is not very clear what the thread access semantics are supposed to be for the VFS. Can
someone explain what the intended thread access patterns are supposed to be (i.e. which
classes are supposed to be multi-thread accessable, and which ones are supposed to be
allocated and used by a single thread)? Assuming the cases where there is some form of
locking are correct, it is not consistently applied, or there are other races.
As an example AbstractVFSContext uses a synchronizedMap (previously a CHM) to store
temporary information. However, all other fields on this class are mutable, yet not
guarded or volatile (vfs, rootPeer, and options). Also, now that this class is using a
synchronized map, the iterator usage by getTempInfos and cleanupTemp is not safe, since
synchronized maps do not protect iterators. It looks like switching to a
ConcurrentSkipListMap would solve this particular safety issue, and provide faster search
performance (it's an O(log n) sorted map). It is JDK 6 but we can pull in the source
to common-core (public domain). However CHM could be restored if putIfAbsent() was used.
A number of the classes which extend AbstractVFSContext have guard issues on their fields
as well (FileSystemContext, TempContext, ZipEntryContext, AbstractVirtualFileHandler (and
its subclases), etc).
There are also cases of non-volatile double checked-locking (VirtualFile, DefaultOptions,
VFSCacheFactory)
Somewhat related but not a thread safety issue (yet), is the odd interaction between the
VFS and the cache policy code in common-core. The default thread safety flag for
IterableTimed is true, which is logged, however the policy is constructed with it being
false. It turns out that the policy does synchronization on some methods regardless of the
setting, and it looks like the VFSCache stuff uses locking in the calling methods, so it
seems to work out ok.
View the original post :
http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4226025#...
Reply to the post :
http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&a...