[Cronet] Native URLRequest API doesn't support direct executors |
|||
Issue descriptionNative URLRequst API doesn't properly support direct executor, which invokes app callback directly on the network thread. The invocation is done with lock held, so if app calls URLRequest method, which attempts to acquire the lock, then CHECK is triggered as Chrome native lock don't allow reentrancy.
,
Aug 21
,
Aug 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45bddde236c6e486c0a3e10c69d2ebf6ffd9942a commit 45bddde236c6e486c0a3e10c69d2ebf6ffd9942a Author: Paul Jensen <pauljensen@chromium.org> Date: Wed Aug 29 17:09:48 2018 [Cronet] Fix two native API bugs 1. TestUrlRequestCallback had a race between destruction and operations performed after SignalDone(). SignalDone() can cause the TestUrlRequestCallback to be deleted by the test, so doing anything that references the "this" pointer after SignalDone() is risking a use-after-free. 2. When Cronet_UrlRequest_Read() is called on a UrlRequest in the "done" state it returns success, but doesn't free the buffer passed in even though it should take ownership the buffer. This fixes a ASAN failure when direct executors are used. Bug: 869402 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I544a40fceb3f40c753045f617f6cf7ba937fe556 Reviewed-on: https://chromium-review.googlesource.com/1194955 Reviewed-by: Misha Efimov <mef@chromium.org> Commit-Queue: Paul Jensen <pauljensen@chromium.org> Cr-Commit-Position: refs/heads/master@{#587167} [modify] https://crrev.com/45bddde236c6e486c0a3e10c69d2ebf6ffd9942a/components/cronet/native/test/test_url_request_callback.cc [modify] https://crrev.com/45bddde236c6e486c0a3e10c69d2ebf6ffd9942a/components/cronet/native/url_request.cc
,
Aug 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6a4cffcaf7ae66c838c96745f0af986a9b0f763a commit 6a4cffcaf7ae66c838c96745f0af986a9b0f763a Author: Paul Jensen <pauljensen@chromium.org> Date: Wed Aug 29 18:25:59 2018 [Cronet] Allow direct executor for native API This is accomplished by avoiding holding Cronet_UrlRequestImpl::lock_ while issuing callbacks. Holding the lock is only necessary while modifying state that should be processed in an atomic fashion. The Cronet_UrlRequestImpl will not be deleted until after final callbacks have been run so there is no chance of a use-after-free. Bug: 869402 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;luci.chromium.try:ios-simulator-cronet Change-Id: Ie851b57296c394e6f098022a7930d65f62f7b0c6 Reviewed-on: https://chromium-review.googlesource.com/1183625 Reviewed-by: Misha Efimov <mef@chromium.org> Cr-Commit-Position: refs/heads/master@{#587203} [modify] https://crrev.com/6a4cffcaf7ae66c838c96745f0af986a9b0f763a/components/cronet/cronet_url_request.h [modify] https://crrev.com/6a4cffcaf7ae66c838c96745f0af986a9b0f763a/components/cronet/native/test/test_url_request_callback.cc [modify] https://crrev.com/6a4cffcaf7ae66c838c96745f0af986a9b0f763a/components/cronet/native/test/test_url_request_callback.h [modify] https://crrev.com/6a4cffcaf7ae66c838c96745f0af986a9b0f763a/components/cronet/native/test/url_request_test.cc [modify] https://crrev.com/6a4cffcaf7ae66c838c96745f0af986a9b0f763a/components/cronet/native/url_request.cc
,
Aug 29
|
|||
►
Sign in to add a comment |
|||
Comment 1 by mef@chromium.org
, Jul 31