[jira] [Updated] (SENTRY-1683) MetastoreCacheInitializer has a race condition in handling results list

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

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

Alexander Kolbasov updated SENTRY-1683:
---------------------------------------
       Resolution: Fixed
    Fix Version/s: 1.8.0
           Status: Resolved  (was: Patch Available)

> MetastoreCacheInitializer has a race condition in handling results list
> -----------------------------------------------------------------------
>
>                 Key: SENTRY-1683
>                 URL: https://issues.apache.org/jira/browse/SENTRY-1683
>             Project: Sentry
>          Issue Type: Bug
>          Components: Sentry
>    Affects Versions: 1.8.0
>            Reporter: Alexander Kolbasov
>            Assignee: Alexander Kolbasov
>             Fix For: 1.8.0
>
>         Attachments: SENTRY-1683.001.patch, SENTRY-1683.001.patch
>
>
> The {{MetastoreCacheInitializer}} has the following logic:
> 1) It creates a list of futures to hold results of executors ({{results}})
> 2) {{CreateInitialUpdate() }}does this:
> {code}
>   UpdateableAuthzPaths createInitialUpdate() throws
>           Exception {
>     PathsUpdate tempUpdate = new PathsUpdate(-1, false);
>     List<String> allDbStr = hmsHandler.get_all_databases();
>     for (String dbName : allDbStr) {
>       Callable<CallResult> dbTask = new DbTask(tempUpdate, dbName);
>       results.add(threadPool.submit(dbTask));
>     }
>     while (taskCounter.get() > 0) {
>       Thread.sleep(1000);
>       // Wait until no more tasks remain
>     }
>     for (Future<CallResult> result : results) {
>       CallResult callResult = result.get();
>       // Fail the HMS startup if tasks are not all successful and
>       // fail on partial updates flag is set in the config.
>       if (!callResult.getSuccessStatus() && failOnRetry) {
>         throw callResult.getFailure();
>       }
>     }
>     authzPaths.updatePartial(Lists.newArrayList(tempUpdate),
>             new ReentrantReadWriteLock());
>     return authzPaths;
>   }
> {code}
> Note that in both cases the results list is accessed without holding any locks. 
> But the list is also updated from tasks themselves:
> {code}
>   class DbTask extends BaseTask {
>     @Override
>     public void doTask() throws TException, SentryMalformedPathException {
>      ...
>       List<String> allTblStr = hmsHandler.get_all_tables(dbName);
>       for (int i = 0; i < allTblStr.size(); i += maxTablesPerCall) {
>         synchronized (results) {
>           results.add(threadPool.submit(tableTask));
>         }
>       }
> {code}
> There is some synchronization which uses taskCounter so the second walk of the list happens when all tasks are complete, but the first walk isn't thread-safe  - the list may be updated while initial tasks are created there. Note that some access are synchronized and some are not.



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