groovy
  1. groovy
  2. GROOVY-4292

ClassHelper.ClassHelperCache#classCache is not thread-safe

    Details

    • Number of attachments :
      0

      Description

      When parsing Groovy scripts from multiple threads at a time, the call to WeakHashMap.get() may never return due to internal corruption due to the concurrent access. Since IntelliJ IDEA does its GDSL parsing in parallel threads, this blocks it completely.

        Activity

        Hide
        blackdrag blackdrag added a comment -

        No wonder, mos of the compiler infrastructure is designed for single thread usage only. I would have suggested to use makeWithoutCaching, but that ends in makeCached too, which uses the map. I think that must be changed then.

        As for a workaround... is it impossible to do some kind of warmup?

        Show
        blackdrag blackdrag added a comment - No wonder, mos of the compiler infrastructure is designed for single thread usage only. I would have suggested to use makeWithoutCaching, but that ends in makeCached too, which uses the map. I think that must be changed then. As for a workaround... is it impossible to do some kind of warmup?
        Hide
        Peter Gromov added a comment -

        As a workaround, we'll now do the parsing in a single thread. But that may take quite long. A warmup could help, if I knew exactly what to invoke to ensure everything's warmed up.

        If it were IDEA sources, I'd just replace WeakHashMap with ConcurrentWeakHashMap, as we have one

        Show
        Peter Gromov added a comment - As a workaround, we'll now do the parsing in a single thread. But that may take quite long. A warmup could help, if I knew exactly what to invoke to ensure everything's warmed up. If it were IDEA sources, I'd just replace WeakHashMap with ConcurrentWeakHashMap, as we have one
        Hide
        blackdrag blackdrag added a comment -

        the warmup would have to contain all the ClassHelper#make calls you usually do. The problem cannot be there for the static stuff that is in that class, since the WeakHashMap may not be able to handle concurrent writes, but concurrent reads are no problem

        Show
        blackdrag blackdrag added a comment - the warmup would have to contain all the ClassHelper#make calls you usually do. The problem cannot be there for the static stuff that is in that class, since the WeakHashMap may not be able to handle concurrent writes, but concurrent reads are no problem
        Hide
        Peter Gromov added a comment -

        The problem is that I don't do any ClassHelper#make calls. I just say new GroovyShell().parse(text) for different script texts. So it's not clear to me which classes should be cached in the map to account for any possible script text.

        Show
        Peter Gromov added a comment - The problem is that I don't do any ClassHelper#make calls. I just say new GroovyShell().parse(text) for different script texts. So it's not clear to me which classes should be cached in the map to account for any possible script text.
        Hide
        blackdrag blackdrag added a comment -

        I replaced the map with an internal concurrent version from our runtime

        Show
        blackdrag blackdrag added a comment - I replaced the map with an internal concurrent version from our runtime

          People

          • Assignee:
            blackdrag blackdrag
            Reporter:
            Peter Gromov
          • Votes:
            9 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: