Concurrency problems in multi resolve in PHPStorm 6.0.2

Hi,

I am developing a custom language plugin for PHPStorm which resolves variables in the custom language to class methods in PHP. Everything was working great but since updating to PHPStorm 6.0.2 I've started to get duplication problems and concurrency errors.
The first problem that showed up was that when I ctrl+clicked a variable the same result would show up several times in the result list. I fixed this by adding some checks but it seemed weird that it was occurring at all.
After this I started to recieve errors about concurrent modification. I changed my list to CopyOnWriteArrayList which I thought fixed it but testing my code again today it threw the same concurrency error.

This is my code for getting the resolve results. I'm getting the error on iteratorList = new CopyOnWriteArrayList<PhpClass>(classes);

     public static List<ResolveResult> getFieldMethodResolverResults(Project project, String key) {
          List<ResolveResult> results = new ArrayList<ResolveResult>();
          List<String> checkedClasses = new ArrayList<String>();
          List<String> checkedArrayClasses = new ArrayList<String>();
          CopyOnWriteArrayList<PhpClass> iteratorList;

          PhpIndex phpIndex = PhpIndex.getInstance(project);
          Collection<PhpClass> classes = phpIndex.getAllSubclasses("Object");
          Collection<PhpClass> extensionClasses = phpIndex.getAllSubclasses("Extension");
          classes.addAll(extensionClasses);
          iteratorList = new CopyOnWriteArrayList<PhpClass>(classes);

          for (PhpClass phpClass : iteratorList) {
               if (phpClass != null) {
                    Method phpMethod = phpClass.findOwnMethodByName(key);
                    if (phpMethod == null) phpMethod = phpClass.findOwnMethodByName("get"+key);
                    if (phpMethod != null && !checkedClasses.contains(phpClass.getName())) {
                         checkedClasses.add(phpClass.getName());
                         results.add(new PsiElementResolveResult(phpMethod));
                    }

                    PsiElement[] arraySearches = {
                         phpClass.findOwnFieldByName("db", false),
                         phpClass.findOwnFieldByName("has_one", false),
                         phpClass.findOwnFieldByName("has_many", false),
                         phpClass.findOwnFieldByName("many_many", false),
                         phpClass.findOwnFieldByName("belongs_many_many", false)
                    };
                    for (PsiElement arraySearch : arraySearches) {
                         if (arraySearch != null) {
                              PsiElement[] arrayKeys = PsiTreeUtil.collectElements(arraySearch, new PsiElementFilter() {
                                   @Override
                                   public boolean isAccepted(PsiElement element) {
                                        return element instanceof ArrayHashElementImpl;
                                   }
                              });
                              for (PsiElement arrayHash : arrayKeys) {
                                   String childText = arrayHash.getFirstChild().getText();
                                   childText = childText.substring(1, childText.length()-1);
                                   if (childText.equals(key) && !checkedArrayClasses.contains(phpClass.getName())) {
                                        checkedArrayClasses.add(phpClass.getName());
                                        results.add(new PsiElementResolveResult(arrayHash.getFirstChild()));
                                   }
                              }
                         }
                    }
               }
          }
          return results;
     }


Has something changed in the way this is handled?
It seems that the resolve method gets called multiple times concurrently when I'm holding down ctrl and hovering the variable but I'm way out of my depth here.
I've read that I could do this in a synchronized block but I'm afraid it would kill it performance wise.
Also since I can't find PHPStorm 6.0.1 to download I can't go back and test in the older version.

1 comment

On first glance, I'd say you should not modify the result that is returned from phpIndex.getAllSubclasses() (which is cached internally) but instead add it to your own list. Something like this:

Collection<PhpClass> classes = phpIndex.getAllSubclasses("Object");
Collection<PhpClass> extensionClasses = phpIndex.getAllSubclasses("Extension");
iteratorList = new ArrayList<PhpClass>(classes);
iteratorList.addAll(extensionClases);


Most likely, PhpIndex should return unmodifiable collections to avoid this in the first place...

Sascha

0

Please sign in to leave a comment.