[jira] [Comment Edited] (FLINK-6130) Consider calling resourceManager#getTerminationFuture() with lock held

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

    [ https://issues.apache.org/jira/browse/FLINK-6130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15970586#comment-15970586 ] 

mingleizhang edited comment on FLINK-6130 at 4/17/17 1:51 AM:
--------------------------------------------------------------

[~tedyu] Could you please help review the code ? How do you think of this ? Thanks.
{code}
	Future<?> future;
	synchronized (lock) {
		// wait for resource manager to finish
		 future = (Future<?>) resourceManager.getTerminationFuture();
	}
	Object object = future.get();
	// everything started, we can wait until all is done or the process is killed
	LOG.info("YARN Application Master finished" + object.toString());
{code}


was (Author: mingleizhang):
[~tedyu] Could you please help review the code ? How do you think of this ? Thanks.
{code}
	Future<?> future;
			synchronized (lock) {
				// wait for resource manager to finish
				 future = (Future<?>) resourceManager.getTerminationFuture();
			}
			Object object = future.get();
			// everything started, we can wait until all is done or the process is killed
			LOG.info("YARN Application Master finished" + object.toString());
{code}

> Consider calling resourceManager#getTerminationFuture() with lock held
> ----------------------------------------------------------------------
>
>                 Key: FLINK-6130
>                 URL: https://issues.apache.org/jira/browse/FLINK-6130
>             Project: Flink
>          Issue Type: Bug
>            Reporter: Ted Yu
>            Assignee: mingleizhang
>            Priority: Minor
>
> In YarnFlinkApplicationMasterRunner#runApplicationMaster() :
> {code}
>       synchronized (lock) {
>         LOG.info("Starting High Availability Services");
> ...
>       }
>       // wait for resource manager to finish
>       resourceManager.getTerminationFuture().get();
> {code}
> resourceManager#getTerminationFuture() is called without holding lock.
> We should store the value returned from resourceManager#getTerminationFuture() inside the synchronized block.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)