Skip to content

Use std::unique_ptr to manage memory #23168

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
merged 3 commits into from
Dec 13, 2021
Merged

Use std::unique_ptr to manage memory #23168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4eceb8
use unique_ptr
deanlee Dec 8, 2021
6ba857d
trigger ci
deanlee Dec 8, 2021
8c52453
move down loggerdstate
deanlee Dec 8, 2021
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
25 changes: 8 additions & 17 deletions selfdrive/boardd/boardd.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,38 +208,31 @@ void can_send_thread(std::vector<Panda *> pandas, bool fake_send) {
set_thread_name("boardd_can_send");

AlignedBuffer aligned_buf;
Context * context = Context::create();
SubSocket * subscriber = SubSocket::create(context, "sendcan");
std::unique_ptr<Context> context(Context::create());
std::unique_ptr<SubSocket> subscriber(SubSocket::create(context.get(), "sendcan"));
assert(subscriber != NULL);
subscriber->setTimeout(100);

// run as fast as messages come in
while (!do_exit && check_all_connected(pandas)) {
Message * msg = subscriber->receive();
std::unique_ptr<Message> msg(subscriber->receive());
if (!msg) {
if (errno == EINTR) {
do_exit = true;
}
continue;
}

capnp::FlatArrayMessageReader cmsg(aligned_buf.align(msg));
capnp::FlatArrayMessageReader cmsg(aligned_buf.align(msg.get()));
cereal::Event::Reader event = cmsg.getRoot<cereal::Event>();

//Dont send if older than 1 second
if (nanos_since_boot() - event.getLogMonoTime() < 1e9) {
if (!fake_send) {
for (const auto& panda : pandas) {
panda->can_send(event.getSendcan());
}
if ((nanos_since_boot() - event.getLogMonoTime() < 1e9) && !fake_send) {
for (const auto& panda : pandas) {
panda->can_send(event.getSendcan());
}
}

delete msg;
}

delete subscriber;
delete context;
Copy link
Contributor

@pd0wm pd0wm Dec 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For ZMQ it's important to delete all subscribers before terminating the context, or that might hang forever. It seems like unique pointers are destructed in reverse order of their creation, so that should work out here. Is there any way we can make this order more explicit?

Copy link
Contributor Author

@deanlee deanlee Dec 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can use braces to make it "more explicit", But I think it might be redundant since objects are always destructed in the reverse order of construction (LIFO).

unique_ptr ctx
{
  unique_ptr msg
}

Copy link
Contributor

@pd0wm pd0wm Dec 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess it's fine since it's impossible to create the subscriber before the context.

}

void can_recv_thread(std::vector<Panda *> pandas) {
Expand Down Expand Up @@ -552,7 +545,7 @@ void pigeon_thread(Panda *panda) {
PubMaster pm({"ubloxRaw"});
bool ignition_last = false;

Pigeon *pigeon = Hardware::TICI() ? Pigeon::connect("/dev/ttyHS0") : Pigeon::connect(panda);
std::unique_ptr<Pigeon> pigeon(Hardware::TICI() ? Pigeon::connect("/dev/ttyHS0") : Pigeon::connect(panda));

std::unordered_map<char, uint64_t> last_recv_time;
std::unordered_map<char, int64_t> cls_max_dt = {
Expand Down Expand Up @@ -620,8 +613,6 @@ void pigeon_thread(Panda *panda) {
// 10ms - 100 Hz
util::sleep_for(10);
}

delete pigeon;
}

int main(int argc, char *argv[]) {
Expand Down
12 changes: 4 additions & 8 deletions selfdrive/locationd/ubloxd.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,22 @@ int main() {

PubMaster pm({"ubloxGnss", "gpsLocationExternal"});

Context * context = Context::create();
SubSocket * subscriber = SubSocket::create(context, "ubloxRaw");
std::unique_ptr<Context> context(Context::create());
std::unique_ptr<SubSocket> subscriber(SubSocket::create(context.get(), "ubloxRaw"));
assert(subscriber != NULL);
subscriber->setTimeout(100);


while (!do_exit) {
Message * msg = subscriber->receive();
std::unique_ptr<Message> msg(subscriber->receive());
if (!msg) {
if (errno == EINTR) {
do_exit = true;
}
continue;
}

capnp::FlatArrayMessageReader cmsg(aligned_buf.align(msg));
capnp::FlatArrayMessageReader cmsg(aligned_buf.align(msg.get()));
cereal::Event::Reader event = cmsg.getRoot<cereal::Event>();
auto ubloxRaw = event.getUbloxRaw();

Expand All @@ -58,11 +58,7 @@ int main() {
}
bytes_consumed += bytes_consumed_this_time;
}
delete msg;
}

delete subscriber;
delete context;

return 0;
}
10 changes: 4 additions & 6 deletions selfdrive/loggerd/loggerd.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,14 @@ void loggerd_thread() {
} QlogState;
std::unordered_map<SubSocket*, QlogState> qlog_states;

LoggerdState s;
s.ctx = Context::create();
Poller * poller = Poller::create();
std::unique_ptr<Context> ctx(Context::create());
std::unique_ptr<Poller> poller(Poller::create());

// subscribe to all socks
for (const auto& it : services) {
if (!it.should_log) continue;

SubSocket * sock = SubSocket::create(s.ctx, it.name);
SubSocket * sock = SubSocket::create(ctx.get(), it.name);
assert(sock != NULL);
poller->registerSocket(sock);
qlog_states[sock] = {
Expand All @@ -203,6 +202,7 @@ void loggerd_thread() {
};
}

LoggerdState s;
// init logger
logger_init(&s.logger, "rlog", true);
logger_rotate(&s);
Expand Down Expand Up @@ -266,6 +266,4 @@ void loggerd_thread() {

// messaging cleanup
for (auto &[sock, qs] : qlog_states) delete sock;
delete poller;
delete s.ctx;
}
1 change: 0 additions & 1 deletion selfdrive/loggerd/loggerd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ const LogCameraInfo qcam_info = {
};

struct LoggerdState {
Context *ctx;
LoggerState logger = {};
char segment_path[4096];
std::mutex rotate_lock;
Expand Down