Skip to content

Fixed memory leaks with http/3 #9342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bryancall merged 2 commits into from
Feb 7, 2023
Merged

Fixed memory leaks with http/3 #9342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95fbfad
Fixed memory leaks with http/3
bryancall Jan 31, 2023
387d3e8
Give ownership of the application to the quic stream manager
bryancall Feb 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions iocore/net/QUICNet.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ QUICPollCont::_process_packet(QUICPollEvent *e, NetHandler *nh)
}

// Note: We should free QUICPollEvent here since vc could be freed from other thread.
p->free();
Comment thread
maskit marked this conversation as resolved.
e->free();
}
#else
Expand Down
2 changes: 0 additions & 2 deletions iocore/net/QUICNetVConnection.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1649,7 +1649,6 @@ QUICNetVConnection::_packetize_frames(uint8_t *packet_buf, QUICEncryptionLevel l
max_frame_size = std::min(max_frame_size, this->_maximum_stream_frame_data_size());

bool probing = false;
int frame_count = 0;
size_t len = 0;
Ptr<IOBufferBlock> first_block = make_ptr<IOBufferBlock>(new_IOBufferBlock());
Ptr<IOBufferBlock> last_block = first_block;
Expand Down Expand Up @@ -1688,7 +1687,6 @@ QUICNetVConnection::_packetize_frames(uint8_t *packet_buf, QUICEncryptionLevel l
break;
}

++frame_count;
probing |= frame->is_probing_frame();
if (frame->is_flow_controlled()) {
int ret = this->_remote_flow_controller->update(this->_stream_manager->total_offset_sent());
Expand Down
7 changes: 7 additions & 0 deletions iocore/net/quic/QUICApplicationMap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ class QUICApplicationMap
void set(QUICStreamId id, QUICApplication *app);
void set_default(QUICApplication *app);
QUICApplication *get(QUICStreamId id);
~QUICApplicationMap()
{
for (auto it = _map.begin(); it != _map.end(); ++it) {
delete it->second;
}
delete _default_app;
}

private:
std::map<QUICStreamId, QUICApplication *> _map;
Expand Down
20 changes: 8 additions & 12 deletions iocore/net/quic/test/test_QUICStreamManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ TEST_CASE("QUICStreamManager_NewStream", "[quic]")
QUICEncryptionLevel level = QUICEncryptionLevel::ONE_RTT;
QUICApplicationMap app_map;
MockQUICConnection connection;
MockQUICApplication mock_app(&connection);
app_map.set_default(&mock_app);
app_map.set_default(new MockQUICApplication(&connection));
MockQUICConnectionInfoProvider cinfo_provider;
QUICStreamManagerImpl sm(&context, &app_map);

Expand Down Expand Up @@ -101,8 +100,7 @@ TEST_CASE("QUICStreamManager_first_initial_map", "[quic]")
QUICEncryptionLevel level = QUICEncryptionLevel::ONE_RTT;
QUICApplicationMap app_map;
MockQUICConnection connection;
MockQUICApplication mock_app(&connection);
app_map.set_default(&mock_app);
app_map.set_default(new MockQUICApplication(&connection));
MockQUICConnectionInfoProvider cinfo_provider;
QUICStreamManagerImpl sm(&context, &app_map);
std::shared_ptr<QUICTransportParameters> local_tp = std::make_shared<QUICTransportParametersInEncryptedExtensions>();
Expand All @@ -127,8 +125,7 @@ TEST_CASE("QUICStreamManager_total_offset_received", "[quic]")
QUICEncryptionLevel level = QUICEncryptionLevel::ONE_RTT;
QUICApplicationMap app_map;
MockQUICConnection connection;
MockQUICApplication mock_app(&connection);
app_map.set_default(&mock_app);
app_map.set_default(new MockQUICApplication(&connection));
QUICStreamManagerImpl sm(&context, &app_map);

uint8_t local_tp_buf[] = {
Expand Down Expand Up @@ -182,8 +179,8 @@ TEST_CASE("QUICStreamManager_total_offset_sent", "[quic]")
QUICEncryptionLevel level = QUICEncryptionLevel::ONE_RTT;
QUICApplicationMap app_map;
MockQUICConnection connection;
MockQUICApplication mock_app(&connection);
app_map.set_default(&mock_app);
auto mock_app = new MockQUICApplication(&connection);
app_map.set_default(mock_app);
QUICStreamManagerImpl sm(&context, &app_map);

uint8_t local_tp_buf[] = {
Expand Down Expand Up @@ -232,12 +229,12 @@ TEST_CASE("QUICStreamManager_total_offset_sent", "[quic]")

// total_offset should be a integer in unit of octets
uint8_t frame_buf[4096];
mock_app.send(reinterpret_cast<uint8_t *>(block_1024->buf()), 1024, 0);
mock_app->send(reinterpret_cast<uint8_t *>(block_1024->buf()), 1024, 0);
sm.generate_frame(frame_buf, QUICEncryptionLevel::ONE_RTT, 16384, 16384, 0, 0, nullptr);
CHECK(sm.total_offset_sent() == 1024);

// total_offset should be a integer in unit of octets
mock_app.send(reinterpret_cast<uint8_t *>(block_1024->buf()), 1024, 4);
mock_app->send(reinterpret_cast<uint8_t *>(block_1024->buf()), 1024, 4);
sm.generate_frame(frame_buf, QUICEncryptionLevel::ONE_RTT, 16384, 16384, 0, 0, nullptr);
CHECK(sm.total_offset_sent() == 2048);

Expand All @@ -250,8 +247,7 @@ TEST_CASE("QUICStreamManager_max_streams", "[quic]")
QUICEncryptionLevel level = QUICEncryptionLevel::ONE_RTT;
QUICApplicationMap app_map;
MockQUICConnection connection;
MockQUICApplication mock_app(&connection);
app_map.set_default(&mock_app);
app_map.set_default(new MockQUICApplication(&connection));
QUICStreamManagerImpl sm(&context, &app_map);

uint8_t local_tp_buf[] = {
Expand Down
6 changes: 5 additions & 1 deletion proxy/http3/QPACK.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ QPACK::QPACK(QUICConnection *qc, uint32_t max_header_list_size, uint16_t max_tab
this->_decoder_stream_sending_instructions_reader = this->_decoder_stream_sending_instructions->alloc_reader();
}

QPACK::~QPACK() {}
QPACK::~QPACK()
{
free_MIOBuffer(_encoder_stream_sending_instructions);
free_MIOBuffer(_decoder_stream_sending_instructions);
}

void
QPACK::on_new_stream(QUICStream &stream)
Expand Down