Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-1009

SelfPopulatingList is not thread-safe

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.6.0
    • Component/s: None
    • Labels:
      None

      Description

      When I use drill, I found a bug when calcite parsing sql.
      in org.apache.calcite.rex.RexLocalRef, there is a method named createName(int index), when this method is called distributed, you find that you're not, NAMES may not be {$0 $1 .... $n}, but {$0...$29,$30...$59,$30...}
      NAMES is SelfPopulatingList.class liked Thread-safe list, but unfortunately it's Thread-unsafe list, although addAll() has a lock, this method is Thread-safe, but in method get(int index)

      @Override public String get(int index) {
            for (;;) {
              try {
                return super.get(index);
              } catch (IndexOutOfBoundsException e) {
                if (index < 0) {
                  throw new IllegalArgumentException();
                }
                addAll(
                    fromTo(
                        prefix, size(), Math.max(index + 1, size() * 2)));
              }
            }
          }
      

      method fromTo() is not thread-safe, so it get error. So as you can see, {$30...$59} is added repeatedly.

      bugfix:
      There are several ways to solve this bug.
      one is, add lock before addAll(), seemed like

      @Override public String get(int index) {
            for (;;) {
              try {
                return super.get(index);
              } catch (IndexOutOfBoundsException e) {
                if (index < 0) {
                  throw new IllegalArgumentException();
                }
      /*********lock************/
                addAll(
                    fromTo(
                        prefix, size(), Math.max(index + 1, size() * 2)));
              }
      /*********unlock************/
            }
          }
      

      But there is over design, catch(e) is not good.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                julianhyde Julian Hyde
                Reporter:
                huntersjm huntersjm
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: