Chromium Code Reviews Chromium Code Reviews
Issue
2189913002:
Revert of Substituting legacy protocol with uWeave protocol (Closed)
DescriptionRevert of Substituting legacy protocol with uWeave protocol (patchset #33 id:630001 of https://codereview.chromium.org/2075313002/ )
Reason for revert:
This CL has a memory problem in the unit tests, as reported by DrMemory.
First build showing the error: https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/5364
Note, that build doesn't include this change as there was a problem introduced earlier that caused the whole build to fail.
Error text:
UNADDRESSABLE ACCESS of freed memory: reading 0x069fa848-0x069fa84c 4 byte(s)
# 0 proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance [components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator.cc:33]
# 1 proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody [components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator_unittest.cc:54]
# 2 testing::internal::HandleExceptionsInMethodIfSupported<> [testing\gtest\src\gtest.cc:2458]
Note: @0:05:46.506 in thread 13092
Note: 0x069fa848-0x069fa84c overlaps memory 0x069fa5d8-0x069fa8c0 that was freed here:
Note: # 0 replace_operator_delete_nothrow [d:\drmemory_package\common\alloc_replace.c:2974]
Note: # 1 proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeaveClientConnectionTest_ConnectAfterADelayWhenThrottled_Test::`scalar deleting destructor'
Note: # 2 testing::Test::DeleteSelf_ [testing\gtest\include\gtest\gtest.h:453]
Note: # 3 testing::TestInfo::Run [testing\gtest\src\gtest.cc:2661]
Note: instruction: mov (%eax) -> %edx
Suppression (error hash=#E743E83C1B0FB1CF#):
For more info on using suppressions see http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-error-reports-from-the-
{
UNADDRESSABLE ACCESS
name=<insert_a_suppression_name_here>
*!proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
*!proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
*!testing::internal::HandleExceptionsInMethodIfSupported<>
}
The problem is real, to fix this SetInstanceForTesting should keep the previous factory around somewhere, and there should be a method to clear the testing instance. Or something like that.
Original issue's description:
> Using the uWeave packet generator and receiver to re-implement the BluetoothLowEnergyConnection class
>
> Major changes:
> SendMesssageImpl which instead of using the old protocol, it will just generate the message with the WeavePacketGenerator.
>
> GattCharacteristicValueChanged instead of parsing messages received with the old protocol, will now use the WeavePacketReceiver and just switch on the state of the receiver and take appropriate action.
>
> Design Doc for ProximityAuth uWeave Migration:
> https://docs.google.com/a/google.com/document/d/1HJ1fipxsNQwIxmKs-5_6TjroCAcs6qRhAYvOnIr6mQw/edit?usp=sharing
>
> BUG=617238
>
> Committed: https://crrev.com/f4f0a5cde8eff464de5359281560444ff442f920
> Cr-Commit-Position: refs/heads/master@{#407655}
TBR=khorimoto@chromium.org,tengs@chromium.org,sacomoto@chromium.org,msarda@chromium.org,jingxuy@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=617238
Committed: https://crrev.com/932d272591d6c54513a837f70d0ebb182f5cc19c
Cr-Commit-Position: refs/heads/master@{#408351}
Patch Set 1 #
Messages
Total messages: 7 (3 generated)
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
