LevelDB is incorrectly handling Get-requests (//components/leveldb_proto) |
||
Issue descriptionWhat steps will reproduce the problem? (1) Try to retrieve a non-existing entry from the DB. What is the expected result? Found is set to false (success is still true). What happens instead? Found is set to false (success is still true), but an error message appears in the log. Example: [libprotobuf ERROR ../../third_party/protobuf/src/google/protobuf/message_lite.cc:121] Can't parse message of type "media.CapabilitiesInfo" because it is missing required fields: (cannot determine missing fields for lite message) [5440:5443:0929/102655.932225:1921092541334:WARNING:proto_database_impl.h(203)] Unable to parse leveldb_proto entry Notes: ProtoDatabaseImplTest.TestDBGetSuccess, ProtoDatabaseImplTest.TestDBGetNotFound, and ProtoDatabaseImplTest.TestDBGetFailure tests this behavior, however, it does not have the ability to do as deep an inspection as to inspect what it does to comply with the API.
,
Oct 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c84784ce85d0fada9677e00d95a25b76237b2028 commit c84784ce85d0fada9677e00d95a25b76237b2028 Author: Tommy Nyquist <nyquist@chromium.org> Date: Mon Oct 09 21:14:53 2017 Fix handling of not found entries when invoking LevelDB::Get The ProtoDatabaseImpl of //components/leveldb_proto uses the underlying DB to look for items. Whenever found is set to false, it still tries to parse the entries, which leads to unnecessary error-log messages for parsing. In addition, instead of checking the value of *success, it checks whether there is a pointer to success (which is already DCHECKed in the start of the method). This CL fixes the check of *success and also adds an early bail out if the entry is not found to ensure no parsing happens. |found| is also forcefully set to false if we failed the lookup (i.e. |success| was false). BUG= 772125 Change-Id: Ie23c46e0c0e9d48fbd2746348f19f694d82a0b68 Reviewed-on: https://chromium-review.googlesource.com/703466 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Tommy Nyquist <nyquist@chromium.org> Cr-Commit-Position: refs/heads/master@{#507489} [modify] https://crrev.com/c84784ce85d0fada9677e00d95a25b76237b2028/components/leveldb_proto/proto_database_impl.h
,
Oct 9 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by nyquist@chromium.org
, Oct 5 2017