Skip to content

Commit

Permalink
Revert of Revert of Substituting legacy protocol with uWeave protocol…
Browse files Browse the repository at this point in the history
… (patchset #1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to be reverted is fixed.

First build showing the error:
n/a

Error text:
n/a

Original issue's description:
>Revert 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.

BUG=617238

Review-Url: https://codereview.chromium.org/2188053003
Cr-Commit-Position: refs/heads/master@{#409618}
  • Loading branch information
jingxuy authored and Commit bot committed Aug 3, 2016
1 parent d01a2d6 commit 2af35dd
Show file tree
Hide file tree
Showing 10 changed files with 2,022 additions and 12 deletions.
1 change: 1 addition & 0 deletions components/components_tests.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,7 @@
'proximity_auth/ble/bluetooth_low_energy_connection_finder_unittest.cc',
'proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc',
'proximity_auth/ble/bluetooth_low_energy_device_whitelist_unittest.cc',
'proximity_auth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc',
'proximity_auth/ble/bluetooth_low_energy_weave_packet_generator_unittest.cc',
'proximity_auth/ble/bluetooth_low_energy_weave_packet_receiver_unittest.cc',
'proximity_auth/bluetooth_connection_finder_unittest.cc',
Expand Down
2 changes: 2 additions & 0 deletions components/proximity_auth.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
"proximity_auth/ble/bluetooth_low_energy_connection_finder.h",
"proximity_auth/ble/bluetooth_low_energy_device_whitelist.cc",
"proximity_auth/ble/bluetooth_low_energy_device_whitelist.h",
"proximity_auth/ble/bluetooth_low_energy_weave_client_connection.cc",
"proximity_auth/ble/bluetooth_low_energy_weave_client_connection.h",
"proximity_auth/ble/bluetooth_low_energy_weave_defines.h",
"proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc",
"proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.h",
Expand Down
3 changes: 3 additions & 0 deletions components/proximity_auth/ble/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ static_library("ble") {
"bluetooth_low_energy_connection_finder.h",
"bluetooth_low_energy_device_whitelist.cc",
"bluetooth_low_energy_device_whitelist.h",
"bluetooth_low_energy_weave_client_connection.cc",
"bluetooth_low_energy_weave_client_connection.h",
"bluetooth_low_energy_weave_defines.h",
"bluetooth_low_energy_weave_packet_generator.cc",
"bluetooth_low_energy_weave_packet_generator.h",
Expand Down Expand Up @@ -51,6 +53,7 @@ source_set("unit_tests") {
"bluetooth_low_energy_connection_finder_unittest.cc",
"bluetooth_low_energy_connection_unittest.cc",
"bluetooth_low_energy_device_whitelist_unittest.cc",
"bluetooth_low_energy_weave_client_connection_unittest.cc",
"bluetooth_low_energy_weave_packet_generator_unittest.cc",
"bluetooth_low_energy_weave_packet_receiver_unittest.cc",
]
Expand Down
Loading

0 comments on commit 2af35dd

Please sign in to comment.