CODEWITHSUNDEEP
c++segmentation-faultunique-ptr
O.B.
O.B.

Reputation: 39

Segmentation fault with unique_ptr

I am trying to use unique_ptr instead of allocating memory myself. I have the following code:

class Album {
...
public:
    Add(Song* song);
...
}

void func(){
    ...
    std::unique_ptr<Album> album = std::unique_ptr<Album>{new Album()};
    std::unique_ptr<Song> song = std::unique_ptr<Song>{new Song(soundtrack.data(), soundtrack.length())};
    album->Add(song.get());
    ...
}

I get segmentation fault for the line:

album->Add(song.get());

I tried multiple variations to get the pointer, including std::move and make_unique, but maybe I don't understand how unique_ptr works well enough to solve it.

Any ideas?

Upvotes: 2

Views: 8660

Answers (2)

Necktschnagge
Necktschnagge

Reputation: 427

The problem is as follows

class Album {
...
public:
    Add(Song* song);
...
}

void func(){
    ...
    std::unique_ptr<Album> album = std::unique_ptr<Album>{new Album()};
    std::unique_ptr<Song> song = std::unique_ptr<Song>{new Song(soundtrack.data(), soundtrack.length())};
    album->Add(song.get());
    ...
    // Here the object song gets destructed. This means that the underlying Song gets destructed.
    // So right after leaving func() the pointer that was returned by song.get() now points to non-allocated memory containing random bits at worst case.
}

So one possible solution would be...

class Album {
...
public:
    Add(std::unique_ptr<Song> song); // you still need to move song inside Add(...)
...
}

void func(){
    ...
    std::unique_ptr<Album> album = std::unique_ptr<Album>{new Album()};
    std::unique_ptr<Song> song = std::unique_ptr<Song>{new Song(soundtrack.data(), soundtrack.length())};
    album->Add(std::move(song)); //here song is intended  to be moved inside Add(...)
    ...
    // song points to nullptr here.
}

Upvotes: 1

Xantopp
Xantopp

Reputation: 74

The code you provided compiles and runs fine - thus there must be a problem in the part you didnot provide - I suspect code inside Add() or it's returntype, or some later use of the pointers as necktschnagge suspected. Working example is on gdbonline:
https://onlinegdb.com/r1oyXGK2S

First of all I ask the question, what is the advantage you'll like to achive by use of std::unique_ptr. Consider that a unique pointer does not guarantee to have a pointee - inside Add() you have to check for nullptr! I think from your usage, you do not want to use a std::unique_ptr:

Key is, that a std::unique_ptr has just a unique ownership. either one of those:

  • func()::local scope
  • album::Add()::parameter_scope

ownes it.

Since you didn't use std::move() the ownership remains in func(), and will be destroyed by the end of func(). To avoid this you might as well use song.release() (see cpp-reference).

Upvotes: 1

Related Questions