Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9089 )
Change subject: IMPALA-6215: Removes race when using LibCache. ...................................................................... Patch Set 2: (1 comment) I think the usage LibCacheEntry* interface is analogous to the existing GetSoFunctionPtr() interface. I think in a lot of other places we've exposed handles that automatically close or DCHECK wrapping the internal data structure. I agree that the alternative approach is less error-prone so is worth considering. I don't feel strongly about whether to do it in this patch or defer the work (the use ScopeExitTrigger pattern solves the problem but is a bit clunky). http://gerrit.cloudera.org:8080/#/c/9089/2/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: http://gerrit.cloudera.org:8080/#/c/9089/2/be/src/codegen/llvm-codegen.cc@333 PS2, Line 333: MakeScopeExitTrigger([entry]() { : if (entry != nullptr) LibCache::instance()->DecrementUseCount(entry);} : ); > maybe do this in LibCacheEntry's destructor and use a unique_ptr here Isn't LibCacheEntry shared between all of the callers? -- To view, visit http://gerrit.cloudera.org:8080/9089 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9175085201fe8b11424ab8f88d7b992cb7b0daea Gerrit-Change-Number: 9089 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 24 Jan 2018 01:56:01 +0000 Gerrit-HasComments: Yes
