AsyncFileUtil should clean up base::File objects properly if the callback goes away |
|||
Issue descriptionI'm working on creating a mojo file service for use by Content Decryption Modules running in the utility process. This service calls AsyncFileUtil::CreateOrOpen() when opening the file. Normally the file returned in the CreateOrOpenCallback is simply passed to mojo to send to the client, as the service doesn't do any I/O. However, if mojo destroys the service (due to a connection error, for example), then the callback doesn't run due to a weak_ptr, and the base::File is cleaned up on the services thread, which doesn't allow for blocking calls. Example crash (Linux): [7095:7095:1025/162344.748076:FATAL:thread_restrictions.cc(29)] Check failed: !g_blocking_disallowed.Get().Get(). Function marked as blocking was called from a scope that disallows blocking! If this task is running inside the TaskScheduler, it needs to have MayBlock() in its TaskTraits. Otherwise, consider making this blocking work asynchronous or, as a last resort, you may use ScopedAllowBlocking in a narrow scope. #0 0x0000056297a7 base::debug::StackTrace::StackTrace() #1 0x000005647e41 logging::LogMessage::~LogMessage() #2 0x0000056b6b60 base::AssertBlockingAllowed() #3 0x000005638837 base::File::Close() #4 0x00000562ef09 base::File::~File() #5 0x0000078b2c2d storage::(anonymous namespace)::RunCreateOrOpenCallback() #6 0x0000078b510c _ZN4base8internal7InvokerINS0_9BindStateIPFvPN7storage26FileSystemOperationContextERKNS_17RepeatingCallbackIFvNS_4FileERKNS6_IFvvEEEEEES7_EJNS0_12OwnedWrapperIS4_EESD_EEEFvS7_EE3RunEPNS0_13BindStateBaseEOS7_ #7 0x0000041c32b9 base::internal::ReplyAdapter<>() #8 0x000003b2423b _ZN4base8internal7InvokerINS0_9BindStateIPFvNS_12OnceCallbackIFvN10extensions12_GLOBAL__N_112UnpackResultEEEEPS6_EJS8_NS0_12OwnedWrapperIS6_EEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE #9 0x0000056add3e base::(anonymous namespace)::PostTaskAndReplyRelay::RunReplyAndSelfDestruct()
,
Oct 27 2017
I ended up providing CreateOrOpen() a callback to a static method that takes a weak ptr and the "real" callback. If the weak ptr is null, post a task to free the base::File object. I just tried checking IsCancelled() in my code. (On Windows) It is true if the weak ptr is NULL, so it looks like it will work. I'll try modifying AsyncFileUtil to check, remove the static method method in my code, and try. If it works I'll submit a CL to fix AsyncFileUtil.
,
Oct 27 2017
Thanks! If that doesn't pan out we should update the docs for CreateOrOpen to clarify that the passed callback must dispose of the File on an appropriate sequence.
,
Nov 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4b5dbaf4b2c13bb6da70486401429fa6ad2bc77e commit 4b5dbaf4b2c13bb6da70486401429fa6ad2bc77e Author: John Rummell <jrummell@chromium.org> Date: Fri Nov 03 23:54:46 2017 Clean up base::File objects properly if the callback goes away If the callback has been cancelled (e.g due to weak ptr and that object has been destroyed), run base::File::Close() on the thread that allows I/O. BUG= 778839 TEST=tests using CdmFile still pass Change-Id: I116b1d6068b55b0bd3c429a6dbdcfa9d662c0927 Reviewed-on: https://chromium-review.googlesource.com/747878 Commit-Queue: John Rummell <jrummell@chromium.org> Reviewed-by: Joshua Bell <jsbell@chromium.org> Reviewed-by: Xiaohan Wang <xhwang@chromium.org> Cr-Commit-Position: refs/heads/master@{#513971} [modify] https://crrev.com/4b5dbaf4b2c13bb6da70486401429fa6ad2bc77e/content/browser/media/cdm_file_impl.cc [modify] https://crrev.com/4b5dbaf4b2c13bb6da70486401429fa6ad2bc77e/storage/browser/fileapi/async_file_util_adapter.cc
,
Nov 6 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by jsb...@chromium.org
, Oct 27 2017