7 Replies Latest reply on Apr 17, 2009 10:37 AM by alesj

    VFS thread safety issues

    jason.greene

      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.

        • 1. Re: VFS thread safety issues
          alesj

          You're free to fix any thread or opt issues. ;-)

          There is no concrete logic behind it
          apart from how classes are "naturally" used.
          e.g. single VFSContext per vfs root

          The idea was to solve all FS problems,
          opt kicks in later.

          • 2. Re: VFS thread safety issues
            jason.greene

             

            "alesj" wrote:
            You're free to fix any thread or opt issues. ;-)

            There is no concrete logic behind it
            apart from how classes are used.
            e.g. single VFSContext per vfs root

            The idea was to solve all FS problems,
            opt kicks in later.


            Sure, I just wanted to make sure I got the rules right before I started changing everything. Are all classes and all plugins supposed to support access from different threads? Is anything supposed to be immutable or tied to a single thread?

            • 3. Re: VFS thread safety issues
              dmlloyd

              Just a small clarification. Thread safety is not about optimization, it's about making the appserver not randomly implode. If there's no concrete logic behind how it's supposed to work, then it's broken, and we've got a lot of work to do...

              • 4. Re: VFS thread safety issues
                alesj

                 

                "david.lloyd@jboss.com" wrote:
                If there's no concrete logic behind how it's supposed to work, then it's broken, and we've got a lot of work to do...

                OK, let's not exaggerate. ;-)

                FS is not rocket science, it has a "natural" logic.
                If you have hard time grasping what the code does
                we can set up a conf call and I'll try to describe how it's meant to work.
                Or if Scott left some design wikis behind when he did original architecture.
                Or just simply ask me what some piece does.

                But in general, all code is subject to multi-thread access.
                Single root - e.g. from deploy/ - should be able to handle multiple threads.
                e.g. from ProfileService, HD scanner, JBAS-6330, ...



                • 5. Re: VFS thread safety issues
                  jason.greene

                  Ok great thanks for the info. I'll do some updates to it over the weekend.

                  • 6. Re: VFS thread safety issues
                    alesj

                     

                    "jason.greene@jboss.com" wrote:
                    Ok great thanks for the info.

                    Not really much of an info actually. :-)

                    VFS is really pretty much straight fwd,
                    but as you could see yourself with JBAS-6715 or that past "/" discussion,
                    the devil is in the details, one of them being thread access.

                    "jason.greene@jboss.com" wrote:

                    I'll do some updates to it over the weekend.

                    Just ping me here on this thread if you need any help.

                    Meanwhile I'll port all the trunk changes to proper branches.
                    If I'm able to do this before you start doing your updates,
                    I suggest you do them into 2.1 branch and I'll later port them to trunk.

                    • 7. Re: VFS thread safety issues
                      alesj

                       

                      "alesj" wrote:

                      If I'm able to do this before you start doing your updates,
                      I suggest you do them into 2.1 branch and I'll later port them to trunk.

                      This is done, current branch 2.1 == trunk.