zentol commented on PR #21137: URL: https://github.com/apache/flink/pull/21137#issuecomment-1302209878
> Well, that should also work. I don't see much differences compared to the current approach, except for more code changes. But I'm not opposed to it. One advantage of my proposal is that you can't run into TOCTOU races. If the state check runs as part of the sequential operation we are sure that it only runs if we're actually still running by the time the executor processes is. As is, you check that its currently running, schedule an operation, but by the time the operation runs we may have already shut down. The second main defect is that some calls prepared in closeAsync may _or may not_ run under the lock. That's something we must fix in any case (and is the actual bug fix!); the introduction of the executor could be a means to that end, or just be a refactoring. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
