Github user ilooner commented on the issue:
https://github.com/apache/drill/pull/1105
@vrozov The main issue is that InnerNext and Close should not execute
concurrently. Even when we are using AtomicReferences and volatile the
following sequence of events could happen which could cause a memory leak:
1. Let's say there are two Threads. The **Close thread** which starts at
the beginning of the close method, and the **Next thread** which starts at the
beginning of the innerNext method.
1. Now let's say the **Next Thread** runs and checks **ok**. Since close
has not been called yet **ok** is true.
1. Now the **Next Thread** is after the ok check.
1. The **Close thread** now starta executing. And the close thread clears
the partitioner.
1. Now after the partitioner is cleared the **Next Thread** can resume
executing. If the next thread receives an OK_SCHEMA he will allocate a new
partitioner. Since the OK_SCHEMA message may include records the partitioner
may partition some data as well.
1. Now the **Close thread** is done, but there is a partitioner that has
not been closed, and we will leak memory.
In order to property solve this problem we need to make sure that the
innerNext and close methods are mutually exclusive so the above scenario can
never happen. The easiest way to do that is to use the synchronized key word.
If we use the synchronized keyword then we don't have to use volatile or atomic
reference.
Also as a side note using synchronized will probably be more efficient
since a cache flush would only be triggered at the start of the innerNext and
close method. Alternatively if we used volatile and AtomicReference a cache
flush would be triggered every time we accessed the ok and partitioner
variables.
---
