Skip to content
Snippets Groups Projects

Update smart pointer exercise.

Merged Stephan Hageboeck requested to merge shageboe/cpluspluscourse:smartPtr into master
5 files
+ 251
101
Compare changes
  • Side-by-side
  • Inline
Files
5
#include <algorithm>
#include <algorithm>
#include <cstdlib>
#include <cstdlib>
#include <ctime>
#include <ctime>
 
#include <functional>
 
#include <stdexcept>
 
#include <iostream>
#include <numeric>
#include <numeric>
#include <vector>
#include <vector>
#include <memory>
#include <memory>
@@ -18,39 +21,50 @@
@@ -18,39 +21,50 @@
/* --------------------------------------------------------------------------------------------
/* --------------------------------------------------------------------------------------------
* 1: Fix the leak using a smart pointer.
* 1: Always use smart pointers when you use new.
*
*
* Note that the arguments of sumEntries() don't need to change, as it has only read access.
* A frequent source of leaks is a function that terminates earlier than the programmer thought.
 
*
 
* - Fix the leak using a smart pointer.
 
* - The arguments of sumEntries() don't need to change, as it has only read access.
* --------------------------------------------------------------------------------------------
* --------------------------------------------------------------------------------------------
*/
*/
// Declare a function to do something with the data. No need to change it.
// Declare a function to do something with the data. No need to change it.
double sumEntries(const double* data, std::size_t size) {
double sumEntries(const double* data, std::size_t size) {
 
if (size > 200)
 
throw std::invalid_argument("I only want to sum 200 numbers or less.");
 
return std::accumulate(data, data + size, 0);
return std::accumulate(data, data + size, 0);
}
}
// Often, data are owned by one entity, and only used by others. Fix the leak.
// Often, data are owned by one entity, and only used by others. Fix the leak.
void problem1() {
void doStuffWithData() {
constexpr std::size_t arraySize = 10000;
constexpr std::size_t arraySize = 10000;
auto data = std::make_unique<double[]>(arraySize);
auto data = std::make_unique<double[]>(arraySize);
// Fill array with random stuff.
for (std::size_t i = 0; i < arraySize; ++i) {
data[i] = static_cast<double>(rand()) / RAND_MAX;
}
sumEntries(data.get(), arraySize);
sumEntries(data.get(), arraySize);
}
}
 
void problem1() {
 
try {
 
doStuffWithData();
 
} catch (std::exception& e) {
 
std::cerr << "problem1(): Do stuff with data terminated with exception:\n" << e.what()
 
<< "\n We may have a memory leak now." << std::endl;
 
}
 
}
 
/* --------------------------------------------------------------------------------------------
/* --------------------------------------------------------------------------------------------
* 2: Storing unique_ptr in collections.
* 2: Storing unique_ptr in collections.
*
*
* Often, one has to store pointers to objects in collections. Fix the memory leaks using unique_ptr.
* Often, one has to store pointers to objects in collections. Fix the memory leaks using unique_ptr.
 
*
* Notes:
* Notes:
* - Factory functions should return objects either directly or using smart pointers, so users are forced to
* - Factory functions should return objects either directly or using smart pointers.
* sort out object ownership properly. Fix the return type of the factory function.
* This is good practice, because it clearly shows who owns an object. Fix the return type of the factory function.
* - The vector should own the objects, so try to store them using smart pointers.
* - The vector should own the objects, so try to store them using smart pointers.
* - Since the visitor function doesn't accept smart pointers, find a solution to pass the objects.
* - Since the visitor function doesn't accept smart pointers, find a solution to pass the objects.
* Note that this works without shared_ptr!
* Note that this works without shared_ptr!
@@ -64,12 +78,15 @@ struct LargeObject {
@@ -64,12 +78,15 @@ struct LargeObject {
// A factory function to create large objects.
// A factory function to create large objects.
std::unique_ptr<LargeObject> createLargeObject() {
std::unique_ptr<LargeObject> createLargeObject() {
return std::make_unique<LargeObject>();
auto object = std::make_unique<LargeObject>();
// or
// Do more setting up of object here
// return std::unique_ptr<LargeObject>(new LargeObject());
// ...
 
 
return object;
}
}
// A function to do something with the objects.
// A function to do something with the objects.
 
// Note that since we don't own the object, we don't need a smart pointer as argument.
void visitLargeObject(LargeObject* object) {
void visitLargeObject(LargeObject* object) {
object->fData[0] = 1.;
object->fData[0] = 1.;
}
}
@@ -79,7 +96,10 @@ void problem2() {
@@ -79,7 +96,10 @@ void problem2() {
for (unsigned int i=0; i < 10; ++i) {
for (unsigned int i=0; i < 10; ++i) {
auto newObj = createLargeObject();
auto newObj = createLargeObject();
largeObjects.push_back(std::move(newObj)); // Can only have one copy, so need to "give up" newObj by moving it.
largeObjects.push_back(std::move(newObj)); // Can only have one copy, so need to "give up" newObj by moving it into the vector.
 
 
// Alternatively:
 
// largeObject.push_back(createLargeObject());
}
}
for (const auto& obj : largeObjects) {
for (const auto& obj : largeObjects) {
@@ -98,7 +118,7 @@ void problem2() {
@@ -98,7 +118,7 @@ void problem2() {
* Here is an example of a completely messed up ownership model. It leaks about 1/10 of the times
* Here is an example of a completely messed up ownership model. It leaks about 1/10 of the times
* it is invoked.
* it is invoked.
* - Verify this by running it in a loop using a command like:
* - Verify this by running it in a loop using a command like:
* CONT=true; while $CONT; do valgrind --leak-check=full --track-origins=yes ./smartPointers 2>&1 | grep -B 5 -A 5 problem3 && CONT=false; done
* while true; do valgrind --leak-check=full --track-origins=yes ./smartPointers 2>&1 | grep -B 5 -A 5 problem3 && exit 1; done
* - Fix the ownership model using shared_ptr!
* - Fix the ownership model using shared_ptr!
* - Convert the vectors to holding shared_ptr.
* - Convert the vectors to holding shared_ptr.
* - Fix the arguments of the functions.
* - Fix the arguments of the functions.
@@ -152,61 +172,113 @@ void problem3() {
@@ -152,61 +172,113 @@ void problem3() {
for (const auto& elm : objVector) {
for (const auto& elm : objVector) {
processElement(elm.get());
processElement(elm.get());
}
}
 
 
// Destruction happens automatically!
}
}
/* --------------------------------------------------------------------------------------------
/* --------------------------------------------------------------------------------------------
* 4: Smart pointers to solve ownership problems in class members.
* 4: Smart pointers as class members.
* *** This one is difficult. ***
*
*
* Class members that are pointers can become a problem.
* Class members that are pointers can quickly become a problem.
* Firstly, if only raw pointers are used, the intended ownerships are unclear.
* Firstly, if only raw pointers are used, the intended ownerships are unclear.
* Secondly, it's easy to overlook that a member has to be deleted. Here, we have an example where
* Secondly, it's easy to overlook that a member has to be deleted.
* the leak may not be obvious.
* Thirdly, pointer members usually require to implement copy or move constructors or assignment
 
* operators.
 
* Since C++-11, one can solve those problems using smart pointers.
 
*
 
* 4.1:
 
* The class "Owner" owns some data, but it is broken. If you copy it like in
 
* problem4_1(), you have two pointers pointing to the same data, but both think
 
* that they own the data.
 
* - Comment in problem4_1() in main().
 
* - Verify that it crashes. Try running valgrind ./smartPointers, it should give you some hints as to
 
* what's happening.
 
* - Fix the Owner class by using a shared_ptr for its _largeObj, which we can copy as much as we want.
 
* - Note: Now you even don't need a destructor.
 
*
 
* 4.2: **BONUS**
 
* We go beyond the scope of the lecture now, and use a weak pointer.
 
* These are used to observe a shared_ptr, but unlike the shared_ptr, they don't prevent the deletion
 
* of the underlying object if all shared_ptr go out of scope.
 
* To *use* the observed data, one has to create a shared_ptr from the weak_ptr, so that it is guaranteed that
 
* the underlying object is alive.
 
*
 
* In our case, the observer class wants to observe the data of the owner, but it doesn't need to own it.
 
* To do this, we use a weak pointer.
*
*
* Tasks:
* Tasks:
* - Why do we have a memory leak?
* - Comment in the line in problem4_2() that crashes the program.
* Hint: Use valgrind to track down what exactly leaks here, and where it was allocated.
* - Rewrite the interface of Owner::getData() such that the observer can see the shared_ptr to the large object.
* Hint 2: The author made a mistake when inheriting from Base.
* - Set up the Observer such that it stores a weak pointer that observes the large object.
* - Clarify the ownership using unique_ptr.
* - In Observer::processData(), access the weak pointer, and use the data *only* if the memory is still alive.
* - Fix the mistake that was made when using inheritance.
* Note: What you need is weak_ptr::lock(). Check out the documentation and the example at the bottom:
 
* https://en.cppreference.com/w/cpp/memory/weak_ptr/lock
* --------------------------------------------------------------------------------------------
* --------------------------------------------------------------------------------------------
*/
*/
// This class doesn't have a leak.
class Owner {
class Base {
public:
public:
Base(std::size_t size) :
Owner() :
fData(new double[size]) { }
_largeObj(new LargeObject()) { }
// We *need* to declare this virtual.
std::shared_ptr<const LargeObject> getData() const {
// Otherwise, only the destructor of Base is called
return _largeObj;
// when we derive from this class.
}
virtual ~Base() = default;
private:
private:
std::unique_ptr<double[]> fData; // Do we own this or not? You have to check the destructor.
std::shared_ptr<LargeObject> _largeObj;
};
};
// At first sight, this also doesn't leak.
void problem4_1() {
class Derived : public Base {
std::vector<Owner> owners;
 
 
for (unsigned int i=0; i < 5; ++i) {
 
Owner owner;
 
owners.push_back(owner);
 
}
 
 
// No problem now. Every object is deallocated only once.
 
}
 
 
 
class Observer {
public:
public:
Derived(std::size_t size) :
Observer(const Owner& owner) :
Base(size),
_largeObj(owner.getData()) { }
_moreData(new LargeObject()) { }
 
double processData() const {
 
if (auto data = _largeObj.lock(); data) { // Needs c++-17
 
return data->fData[0];
 
}
 
 
return -1.;
 
}
private:
private:
std::unique_ptr<LargeObject> _moreData;
std::weak_ptr<const LargeObject> _largeObj; // We don't own this.
};
};
// Let's trigger a leak.
void problem4() {
void problem4_2() {
Base baseObject(10000);
// We directly construct 5 owners inside the vector to get around problem4_1:
Derived derivedObject(10000);
std::vector<Owner> owners(5);
std::unique_ptr<Base> whyDoesThisLeak( new Derived(10000) );
std::vector<Observer> observers;
 
 
observers.reserve(owners.size());
 
for (const auto& owner : owners) {
 
observers.emplace_back(owner);
 
}
 
 
// Now let's destroy the owners:
 
owners.clear();
 
 
for (const auto& observer : observers) {
 
observer.processData();
 
}
}
}
@@ -217,5 +289,6 @@ int main() {
@@ -217,5 +289,6 @@ int main() {
problem1();
problem1();
problem2();
problem2();
problem3();
problem3();
problem4();
problem4_1();
 
problem4_2();
}
}
Loading