[jira] [Updated] (PIG-3050) Fix FindBugs multithreading warnings

["Cheolsoo Park (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/PIG-3050?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Cheolsoo Park updated PIG-3050:
-------------------------------

    Attachment: PIG-3050.patch

Attached is a patch that fixes the following issues:
- Mutual static field
{code:title=PhysicalOperator.java}
public static PigProgressable reporter;
{code}
There was a reported race condition due to this static field (For details, see [here|http://search-hadoop.com/m/2OdLNRMwXa2/Intermittent+NullPointerException&subj=Intermittent+NullPointerException]). Since {{reporter}} should be local to thread, I converted it to ThreadLocal.
- Inconsistent synchronization
{code:title=POStream.java}
public Result getNext(Tuple t) throws ExecException {
    ...
    if(initialized) {
       ...
    }
    ...
}
...
public Result getNextHelper(Tuple t) throws ExecException {
    ...
    synchronized(this) {
       ...
       if(!initialized) {
          ...
       }
       ...
       initialized = true;
       ...
    }
}
{code}
Synchronized access to {{initialized}} is performed inside {{getNextHelper()}}, but unsynchronized access was performed inside {{getNext()}}. I added a synchronized getter method and used that method inside {{getNext()}}.
- Incorrect lazy initialization of static field
{code:title=SpillableMemoryManager.java}    
public static SpillableMemoryManager getInstance() {
    if (manager == null) {
        manager = new SpillableMemoryManager();
    }
    return manager;
}
{code}
FindBugs says, "Because the compiler may reorder instructions, threads are not guaranteed to see a completely initialized object if the method can be called by multiple threads." So I declared {{manager}} as volatile.
- Incorrect lazy initialization and update of static field
{code:title=SchemaTupleBackend.java}
public static void initialize(Configuration jConf, PigContext pigContext, boolean isLocal) throws IOException {
    if (stb != null) {
        LOG.warn("SchemaTupleBackend has already been initialized");
    } else {
        SchemaTupleFrontend.lazyReset(pigContext);
        SchemaTupleFrontend.reset();
        stb = new SchemaTupleBackend(jConf, isLocal);
        stb.copyAndResolve();
    }
}
{code}
FindBugs says, "After the field is set, the object stored into that location is further updated. The setting of the field is visible to other threads as soon as it is set. If further accesses in the method that set the field serve to initialize the object, then you have a very serious multithreading bug." So I moved the assignment to the end of the method after all initialization is done.
- Unsynchronized get method, synchronized set method
{code:title=PigHadoopLogger.java}
public synchronized void setReporter(PigStatusReporter rep) {
    this.reporter = rep;
}
public boolean getAggregate() {
    return aggregate;
}
{code}
I made {{getAggregate()}} synchronized.
                
> Fix FindBugs multithreading warnings
> ------------------------------------
>
>                 Key: PIG-3050
>                 URL: https://issues.apache.org/jira/browse/PIG-3050
>             Project: Pig
>          Issue Type: Bug
>    Affects Versions: 0.11
>            Reporter: Cheolsoo Park
>            Assignee: Cheolsoo Park
>             Fix For: 0.12
>
>         Attachments: PIG-3050.patch
>
>
> There was a race condition reported when running Pig in local mode on the user mailing list. This motivated me to fix potential multithreading bugs that can be identified by FindBugs.
> FindBugs identifies the following potential bugs:
> # Mutable static field
> # Inconsistent synchronization
> # Incorrect lazy initialization of static field
> # Incorrect lazy initialization and update of static field
> # Unsynchronized get method, synchronized set method
> There are in total 1153 warnings that FindBugs complains, but they're outside of the scope of this jira.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira