Skip to content

Commit 3fcd7fc

Browse files
committed
Do not use std::vector = {} to release memory
1 parent fd69ffb commit 3fcd7fc

File tree

4 files changed

+57
-14
lines changed

4 files changed

+57
-14
lines changed

src/headerssync.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <pow.h>
88
#include <timedata.h>
99
#include <util/check.h>
10+
#include <util/vector.h>
1011

1112
// The two constants below are computed using the simulation script on
1213
// https://gist.github.com/sipa/016ae445c132cdf65a2791534dfb7ae1
@@ -51,9 +52,9 @@ HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus
5152
void HeadersSyncState::Finalize()
5253
{
5354
Assume(m_download_state != State::FINAL);
54-
m_header_commitments = {};
55+
ClearShrink(m_header_commitments);
5556
m_last_header_received.SetNull();
56-
m_redownloaded_headers = {};
57+
ClearShrink(m_redownloaded_headers);
5758
m_redownload_buffer_last_hash.SetNull();
5859
m_redownload_buffer_first_prev_hash.SetNull();
5960
m_process_all_remaining_headers = false;

src/net.cpp

+11-12
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include <util/threadinterrupt.h>
3636
#include <util/trace.h>
3737
#include <util/translation.h>
38+
#include <util/vector.h>
3839

3940
#ifdef WIN32
4041
#include <string.h>
@@ -899,8 +900,7 @@ void V1Transport::MarkBytesSent(size_t bytes_sent) noexcept
899900
m_bytes_sent = 0;
900901
} else if (!m_sending_header && m_bytes_sent == m_message_to_send.data.size()) {
901902
// We're done sending a message's data. Wipe the data vector to reduce memory consumption.
902-
m_message_to_send.data.clear();
903-
m_message_to_send.data.shrink_to_fit();
903+
ClearShrink(m_message_to_send.data);
904904
m_bytes_sent = 0;
905905
}
906906
}
@@ -1123,8 +1123,8 @@ void V2Transport::ProcessReceivedMaybeV1Bytes() noexcept
11231123
SetReceiveState(RecvState::V1);
11241124
SetSendState(SendState::V1);
11251125
// Reset v2 transport buffers to save memory.
1126-
m_recv_buffer = {};
1127-
m_send_buffer = {};
1126+
ClearShrink(m_recv_buffer);
1127+
ClearShrink(m_send_buffer);
11281128
} else {
11291129
// We have not received enough to distinguish v1 from v2 yet. Wait until more bytes come.
11301130
}
@@ -1184,8 +1184,7 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept
11841184
/*ignore=*/false,
11851185
/*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION));
11861186
// We no longer need the garbage.
1187-
m_send_garbage.clear();
1188-
m_send_garbage.shrink_to_fit();
1187+
ClearShrink(m_send_garbage);
11891188

11901189
// Construct version packet in the send buffer.
11911190
m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION + VERSION_CONTENTS.size());
@@ -1275,7 +1274,7 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept
12751274
// Ignore flag does not matter for garbage authentication. Any valid packet functions
12761275
// as authentication. Receive and process the version packet next.
12771276
SetReceiveState(RecvState::VERSION);
1278-
m_recv_garbage = {};
1277+
ClearShrink(m_recv_garbage);
12791278
break;
12801279
case RecvState::VERSION:
12811280
if (!ignore) {
@@ -1295,9 +1294,9 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept
12951294
Assume(false);
12961295
}
12971296
// Wipe the receive buffer where the next packet will be received into.
1298-
m_recv_buffer = {};
1297+
ClearShrink(m_recv_buffer);
12991298
// In all but APP_READY state, we can wipe the decoded contents.
1300-
if (m_recv_state != RecvState::APP_READY) m_recv_decode_buffer = {};
1299+
if (m_recv_state != RecvState::APP_READY) ClearShrink(m_recv_decode_buffer);
13011300
} else {
13021301
// We either have less than 3 bytes, so we don't know the packet's length yet, or more
13031302
// than 3 bytes but less than the packet's full ciphertext. Wait until those arrive.
@@ -1511,7 +1510,7 @@ CNetMessage V2Transport::GetReceivedMessage(std::chrono::microseconds time, bool
15111510
LogPrint(BCLog::NET, "V2 transport error: invalid message type (%u bytes contents), peer=%d\n", m_recv_decode_buffer.size(), m_nodeid);
15121511
reject_message = true;
15131512
}
1514-
m_recv_decode_buffer = {};
1513+
ClearShrink(m_recv_decode_buffer);
15151514
SetReceiveState(RecvState::APP);
15161515

15171516
return msg;
@@ -1545,7 +1544,7 @@ bool V2Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept
15451544
m_cipher.Encrypt(MakeByteSpan(contents), {}, false, MakeWritableByteSpan(m_send_buffer));
15461545
m_send_type = msg.m_type;
15471546
// Release memory
1548-
msg.data = {};
1547+
ClearShrink(msg.data);
15491548
return true;
15501549
}
15511550

@@ -1577,7 +1576,7 @@ void V2Transport::MarkBytesSent(size_t bytes_sent) noexcept
15771576
// Wipe the buffer when everything is sent.
15781577
if (m_send_pos == m_send_buffer.size()) {
15791578
m_send_pos = 0;
1580-
m_send_buffer = {};
1579+
ClearShrink(m_send_buffer);
15811580
}
15821581
}
15831582

src/test/util_tests.cpp

+25
Original file line numberDiff line numberDiff line change
@@ -1791,4 +1791,29 @@ BOOST_AUTO_TEST_CASE(util_WriteBinaryFile)
17911791
BOOST_CHECK(valid);
17921792
BOOST_CHECK_EQUAL(actual_text, expected_text);
17931793
}
1794+
1795+
BOOST_AUTO_TEST_CASE(clearshrink_test)
1796+
{
1797+
{
1798+
std::vector<uint8_t> v = {1, 2, 3};
1799+
ClearShrink(v);
1800+
BOOST_CHECK_EQUAL(v.size(), 0);
1801+
BOOST_CHECK_EQUAL(v.capacity(), 0);
1802+
}
1803+
1804+
{
1805+
std::vector<bool> v = {false, true, false, false, true, true};
1806+
ClearShrink(v);
1807+
BOOST_CHECK_EQUAL(v.size(), 0);
1808+
BOOST_CHECK_EQUAL(v.capacity(), 0);
1809+
}
1810+
1811+
{
1812+
std::deque<int> v = {1, 3, 3, 7};
1813+
ClearShrink(v);
1814+
BOOST_CHECK_EQUAL(v.size(), 0);
1815+
// std::deque has no capacity() we can observe.
1816+
}
1817+
}
1818+
17941819
BOOST_AUTO_TEST_SUITE_END()

src/util/vector.h

+18
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,22 @@ inline V Cat(V v1, const V& v2)
4949
return v1;
5050
}
5151

52+
/** Clear a vector (or std::deque) and release its allocated memory. */
53+
template<typename V>
54+
inline void ClearShrink(V& v) noexcept
55+
{
56+
// There are various ways to clear a vector and release its memory:
57+
//
58+
// 1. V{}.swap(v)
59+
// 2. v = V{}
60+
// 3. v = {}; v.shrink_to_fit();
61+
// 4. v.clear(); v.shrink_to_fit();
62+
//
63+
// (2) does not appear to release memory in glibc debug mode, even if v.shrink_to_fit()
64+
// follows. (3) and (4) rely on std::vector::shrink_to_fit, which is only a non-binding
65+
// request. Therefore, we use method (1).
66+
67+
V{}.swap(v);
68+
}
69+
5270
#endif // BITCOIN_UTIL_VECTOR_H

0 commit comments

Comments
 (0)