-
Notifications
You must be signed in to change notification settings - Fork 674
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.
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
Added code to make pointer vector be thread safe #1325
Changes from 9 commits
3cf67aa
6fefefb
5ece0be
ddbfe88
5c6c369
03989cc
8dbb970
37dea48
e2addf2
21083fe
070b8a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
#include <stdio.h> | ||
#include <stdint.h> | ||
#include <vector> | ||
#include <mutex> | ||
|
||
/// @file | ||
|
||
|
@@ -12,14 +13,24 @@ | |
*/ | ||
namespace pcpp | ||
{ | ||
/** | ||
* @struct NullMutex | ||
* A struct containing standard mutex operation but does not perform any action. | ||
* This mutex is used when the user decides PointerVector not be thread safe which be default | ||
*/ | ||
struct NullMutex { | ||
void lock() const {} | ||
void unlock() const {} | ||
}; | ||
|
||
/** | ||
* @class PointerVector | ||
* A template class for representing a std::vector of pointers. Once (a pointer to) an element is added to this vector, | ||
* the element responsibility moves to the vector, meaning the PointerVector will free the object once it's removed from the vector | ||
* This class wraps std::vector and adds the capability of freeing objects once they're removed from it | ||
* This class wraps std::vector and adds the capability of freeing objects once they're removed from it. By default NullMutex is used | ||
* but when user wants the PointerVector to be thread safe; they need to pass the mutex as the template argument to PointerVector | ||
*/ | ||
template<typename T> | ||
template<typename T, typename Mutex=pcpp::NullMutex> | ||
class PointerVector | ||
{ | ||
public: | ||
|
@@ -43,6 +54,7 @@ namespace pcpp | |
*/ | ||
~PointerVector() | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
for (auto iter : m_Vector) | ||
{ | ||
delete iter; | ||
|
@@ -57,6 +69,7 @@ namespace pcpp | |
{ | ||
try | ||
{ | ||
std::lock_guard<Mutex> lk(other.m_Mutex); | ||
for (const auto iter : other) | ||
{ | ||
T* objCopy = new T(*iter); | ||
|
@@ -70,7 +83,7 @@ namespace pcpp | |
throw; | ||
} | ||
} | ||
} | ||
} | ||
catch (const std::exception&) | ||
{ | ||
tigercosmos marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (auto obj : m_Vector) | ||
|
@@ -86,6 +99,7 @@ namespace pcpp | |
*/ | ||
void clear() | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
for (auto iter : m_Vector) | ||
{ | ||
delete iter; | ||
|
@@ -97,31 +111,51 @@ namespace pcpp | |
/** | ||
* Add a new (pointer to an) element to the vector | ||
*/ | ||
void pushBack(T* element) { m_Vector.push_back(element); } | ||
void pushBack(T* element) | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
m_Vector.push_back(element); | ||
} | ||
|
||
/** | ||
* Get the first element of the vector | ||
* @return An iterator object pointing to the first element of the vector | ||
*/ | ||
VectorIterator begin() { return m_Vector.begin(); } | ||
VectorIterator begin() | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this protects as much as you would think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the case on normal scenario too even without multi threading environment. we get an iterator and after some time vector reconstructs itself then the above iterator will be undefined. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, tho in a non-multithreaded scenario you don't have the option of another thread invalidating your iterator whenever. Iterators are invalidated in specific documented calls and the sequence of those calls in relation to iterator usage is deterministic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we are planned to use shared_ptr in another PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we want to add this line (and all other similar ones) later when you raise another PR. |
||
return m_Vector.begin(); | ||
} | ||
|
||
/** | ||
* Get the first element of a constant vector | ||
* @return A const iterator object pointing to the first element of the vector | ||
*/ | ||
ConstVectorIterator begin() const { return m_Vector.begin(); } | ||
ConstVectorIterator begin() const | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
muthusaravanan3186 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return m_Vector.begin(); | ||
} | ||
|
||
/** | ||
* Get the last element of the vector | ||
* @return An iterator object pointing to the last element of the vector | ||
*/ | ||
VectorIterator end() { return m_Vector.end(); } | ||
VectorIterator end() | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
muthusaravanan3186 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return m_Vector.end(); | ||
} | ||
|
||
/** | ||
* Get the last element of a constant vector | ||
* @return A const iterator object pointing to the last element of the vector | ||
*/ | ||
ConstVectorIterator end() const { return m_Vector.end(); } | ||
ConstVectorIterator end() const | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
muthusaravanan3186 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return m_Vector.end(); | ||
} | ||
|
||
|
||
//inline size_t size() { return m_Vector.size(); } | ||
|
@@ -130,13 +164,21 @@ namespace pcpp | |
* Get number of elements in the vector | ||
* @return The number of elements in the vector | ||
*/ | ||
size_t size() const { return m_Vector.size(); } | ||
size_t size() const | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
return m_Vector.size(); | ||
} | ||
|
||
/** | ||
* Returns a pointer of the first element in the vector | ||
* @return A pointer of the first element in the vector | ||
*/ | ||
T* front() { return m_Vector.front(); } | ||
T* front() | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
return m_Vector.front(); | ||
muthusaravanan3186 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
* Removes from the vector a single element (position). Once the element is erased, it's also freed | ||
|
@@ -145,6 +187,7 @@ namespace pcpp | |
*/ | ||
VectorIterator erase(VectorIterator position) | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
delete (*position); | ||
return m_Vector.erase(position); | ||
} | ||
|
@@ -158,7 +201,10 @@ namespace pcpp | |
{ | ||
T* result = (*position); | ||
VectorIterator tempPos = position; | ||
tempPos = m_Vector.erase(tempPos); | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
tempPos = m_Vector.erase(tempPos); | ||
} | ||
position = tempPos; | ||
return result; | ||
} | ||
|
@@ -170,6 +216,7 @@ namespace pcpp | |
*/ | ||
T* at(int index) | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
return m_Vector.at(index); | ||
muthusaravanan3186 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
|
@@ -180,11 +227,13 @@ namespace pcpp | |
*/ | ||
const T* at(int index) const | ||
{ | ||
std::lock_guard<Mutex> lk(m_Mutex); | ||
return m_Vector.at(index); | ||
muthusaravanan3186 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
private: | ||
std::vector<T*> m_Vector; | ||
mutable Mutex m_Mutex; | ||
}; | ||
|
||
} // namespace pcpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: space