Skip to content

Commit

Permalink
#575 use unique_ptr to make valgrind happy
Browse files Browse the repository at this point in the history
- While there is no actual memory leak for a static lifetime
  object required for the duration of the program, failure to
  delete the object can result in tooling reporting false positives..

  Anyway, also fixed the slight 'not quite in spec' usage of the
  MEMBER STATIC variables (which are not strictly qualified under
  constant initialization..), reduced the exposed definition,
  and improved comments.
  • Loading branch information
pnstickne committed Nov 23, 2019
1 parent 2d02959 commit 0e8f927
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 20 deletions.
24 changes: 22 additions & 2 deletions src/vt/trace/trace_containers.cc
Original file line number Diff line number Diff line change
@@ -1,12 +1,32 @@
#include "vt/trace/trace_common.h"
#include "vt/trace/trace_containers.h"

#include <memory>

namespace vt { namespace trace {

// These MUST have static-lifetime and be initialized during constant
// initialization as they are modified during dynamic initialization as
// that is when auto-handler events are registered.
// Using a constexpr constructor / initializer list is also relevant.
// - https://en.cppreference.com/w/cpp/language/constant_initialization
static std::unique_ptr<TraceContainerEventClassType> event_type_container_{};
static std::unique_ptr<TraceContainerEventType> event_container_{};

/*static*/ TraceContainerEventClassType*
TraceContainers::event_type_container_;
TraceContainers::getEventTypeContainer(){
if (event_type_container_ == nullptr) {
event_type_container_ = std::make_unique<TraceContainerEventClassType>();
}
return event_type_container_.get();
}

/*static*/ TraceContainerEventType*
TraceContainers::event_container_;
TraceContainers::getEventContainer(){
if (event_container_ == nullptr) {
event_container_ = std::make_unique<TraceContainerEventType>();
}
return event_container_.get();
}

}} //end namespace vt::trace
24 changes: 6 additions & 18 deletions src/vt/trace/trace_containers.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
#include "vt/trace/trace_common.h"
#include "vt/trace/trace_event.h"

#include <cstdint>
#include <unordered_map>

namespace vt { namespace trace {
Expand All @@ -59,25 +58,14 @@ using EventClassType = EventClass;
using TraceContainerEventType = std::unordered_map<TraceEntryIDType, TraceEventType>;
using TraceContainerEventClassType = std::unordered_map<TraceEntryIDType, EventClassType>;

// Container types are created-as-needed, which occurs during
// intialization to avoid intiailization ordering issues.
class TraceContainers {
public:
static TraceContainerEventClassType* getEventTypeContainer(){
if (not event_type_container_)
event_type_container_ = new TraceContainerEventClassType();
return event_type_container_;
}

static TraceContainerEventType* getEventContainer(){
if (not event_container_)
event_container_ = new TraceContainerEventType();
return event_container_;
}

private:
static TraceContainerEventClassType* event_type_container_;
static TraceContainerEventType* event_container_;
/// Returns container of all registered event types (aka parents).
/// The returned pointer is only valid for the current scope.
static TraceContainerEventClassType* getEventTypeContainer();
/// Returns container of all registered event.
/// The returned pointer is only valid for the current scope.
static TraceContainerEventType* getEventContainer();
};

}} //end namespace vt::trace
Expand Down

1 comment on commit 0e8f927

@pnstickne
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lifflander Also, this does not fix some known SEGV's that can occur during event registration. The 'reduce' example is one of several that will not run with tracing on this build. (It is likely related to handlers wrapping operator().)

Please sign in to comment.