[CALCITE-1009] SelfPopulatingList is not thread-safe

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.6.0
• Assignee: Julian Hyde
• Reporter: huntersjm
• Created: 09/Dec/15 03:59
• Updated: 07/Apr/16 19:05
• Resolved: 09/Dec/15 17:55

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.

Issue Links

• is depended upon by: DRILL-4175 IOBE may occur in Calcite RexProgramBuilder when queries are submitted concurrently
• relates to: CALCITE-440 Non-deterministic IndexOutOfBoundsException in PlannerTest.testBushy5