ASF GitHub Bot commented on DRILL-6125:

Github user ilooner commented on the issue:

    @vrozov Thanks for catching this, I believe you are right. 
hasClouseoutThread guarantees innerNext and close won't be called concurrently. 
However, I still believe innerNext and receivingFragmentFinished could be 
called concurrently, since the ControlMessageHandler thread executes 
recievingFragmentFinished. Additionally in rare cases where a limit query is 
cancelled recievingFragmentFinished and close could be called concurrently as 
well. While reflecting on your comments I also saw another issue where the root 
could be blocked on a next call but a Finished event would not cause root to 
    In light of all of this I actually think the **synchronized** is not 
sufficient. We will have to have some way to interrupt the execution of the 
root when we received a finish signal and only close out the resources after 
receivingFragmentFinished has been called. Similarly if we receive a finish 
signal we should ignore any cancellation requests instead of trying to cancel 
and finish simultaneously and vice versa.
    I will rework the solution to address these issues and let you know when I 
have an update.

> PartitionSenderRootExec can leak memory because close method is not 
> synchronized
> --------------------------------------------------------------------------------
>                 Key: DRILL-6125
>                 URL: https://issues.apache.org/jira/browse/DRILL-6125
>             Project: Apache Drill
>          Issue Type: Bug
>    Affects Versions: 1.13.0
>            Reporter: Timothy Farkas
>            Assignee: Timothy Farkas
>            Priority: Minor
>             Fix For: 1.13.0
> PartitionSenderRootExec creates a PartitionerDecorator and saves it in the 
> *partitioner* field. The creation of the partitioner happens in the 
> createPartitioner method. This method get's called by the main fragment 
> thread. The partitioner field is accessed by the fragment thread during 
> normal execution but it can also be accessed by the receivingFragmentFinished 
> method which is a callback executed by the event processor thread. Because 
> multiple threads can access the partitioner field synchronization is done on 
> creation and on when receivingFragmentFinished. However, the close method can 
> also be called by the event processor thread, and the close method does not 
> synchronize before accessing the partitioner field. Since synchronization is 
> not done the event processor thread may have an old reference to the 
> partitioner when a query cancellation is done. Since it has an old reference 
> the current partitioner can may not be cleared and a memory leak may occur.

This message was sent by Atlassian JIRA

Reply via email to