c++ new-handler

Question

I had just put together my first public project: simple(sane) new-handler for c++.

It allocates some reserved memory on start up, then releases it piece by piece. It may also raise a signal when each piece is released. It allows to monitor approaching of low memory conditions and gracefully handle them at the system level while there are still some memory available and avoid raising meaningless std::bac_alloc exceptions.

Please, review.

https://github.com/alex4747-pub/simple_new_handler

#ifndef _INCLUDE_SIMPLE_NEW_HANDLER_H_
#define _INCLUDE_SIMPLE_NEW_HANDLER_H_

#include <csignal>
#include <cstdint>
#include <cstdlib>
#include <cstring>
#include <exception>
#include <limits>
#include <new>
#include <utility>

namespace simple {

class NewHandler {
 public:
  // Initialize the driver and allocate reserved memory blocks
  //
  // If not enough memory allocate as many blocks as possible
  //
  static void Init(size_t finalBlockSize = 0, size_t reservedBlockCount = 0,
                   size_t reservedBlockSize = 0, int signo = 0) {
    GetWorker().Init(finalBlockSize, reservedBlockCount, reservedBlockSize,
                     signo);
  }

  struct State {
    size_t allocated_block_count;
    size_t available_block_count;
  };

  static State GetState() { return GetWorker().GetState(); }

  // Full state
  struct FullState {
    bool init_done;
    int signo;
    size_t final_block_size;
    bool final_block_allocated;
    size_t reserved_block_size;
    size_t reserved_block_count;
    size_t allocated_block_count;
    size_t available_block_count;
  };

  static FullState GetFullState() { return GetWorker().GetFullState(); }

 private:
  class Worker;

  static void Process() { GetWorker().Process(); }

  static Worker& GetWorker() {
    static Worker worker_;
    return worker_;
  }

  class Worker {
   public:
    Worker()
        : init_done_(),
          signo_(),
          final_block_size_(),
          reserved_block_size_(),
          reserved_block_count_(),
          allocated_block_count_(),
          available_block_count_(),
          final_block_(),
          blk_arr_list_() {}

    // Note on concurrency:
    //
    // It is a responsibility of the user to call init()
    // before entering multithreaded environment
    //
    // The environment guarantees that
    // the handler call itself is thread safe.
    //
    // Volatile is good enough for reporting purposes
    // because we do not care about race condition
    // between actual and reported usage
    //
    void Init(size_t finalBlockSize = 0, size_t reservedBlockCount = 0,
              size_t reservedBlockSize = 0, int signo = 0) {
      if (init_done_) {
        // We expect to be done once and it is done
        // more than once we do not care much
        return;
      }

      final_block_size_ = finalBlockSize;

      size_t finalSize =
          (final_block_size_ + sizeof(Blk) - 1) / sizeof(Blk) * sizeof(Blk);

      if (finalSize) {
        final_block_ = new (std::nothrow) Blk[finalSize / sizeof(Blk)];

        if (final_block_) {
          // Assign a value to map allocated block
          final_block_->m_next = 0;
        }
      }

      reserved_block_count_ = reservedBlockCount;

      unsigned int blockLimit = std::numeric_limits<unsigned int>::max();

      if (reserved_block_count_ < blockLimit)
        blockLimit = static_cast<unsigned int>(reserved_block_count_);

      reserved_block_size_ = reservedBlockSize;

      if (reserved_block_count_ && reserved_block_size_) {
        size_t reservedSize = (reserved_block_size_ + sizeof(Blk) - 1) /
                              sizeof(Blk) * sizeof(Blk);

        for (unsigned int ii = 0; ii < blockLimit; ii++) {
          Blk* blkArr = new (std::nothrow) Blk[reservedSize / sizeof(Blk)];

          if (!blkArr) {
            allocated_block_count_ = ii;
            break;
          }

          blkArr[0].m_next = blk_arr_list_;
          blk_arr_list_ = blkArr;
        }

        if (blk_arr_list_ && !allocated_block_count_) {
          // All reserved blocks were allocated
          allocated_block_count_ = blockLimit;
        }

        available_block_count_ = allocated_block_count_;
      }

      signo_ = signo;

      std::set_new_handler(NewHandler::Process);

      init_done_ = true;
    }

    State GetState() const volatile {
      State s;

      memset(&s, 0, sizeof(s));

      if (!init_done_) {
        return s;
      }

      s.allocated_block_count = allocated_block_count_;
      s.available_block_count = available_block_count_;

      return s;
    }

    FullState GetFullState() const volatile {
      FullState s;

      memset(&s, 0, sizeof(s));

      if (!init_done_) {
        return s;
      }

      s.init_done = true;
      s.signo = signo_;
      s.final_block_size = final_block_size_;
      if (final_block_) s.final_block_allocated = true;
      s.reserved_block_size = reserved_block_size_;
      s.reserved_block_count = reserved_block_count_;
      s.allocated_block_count = allocated_block_count_;
      s.available_block_count = available_block_count_;

      return s;
    }

    void Process() {
      Blk* curBlkArr = blk_arr_list_;

      if (curBlkArr) {
        // Release the first avalable block to the process
        // and raise signal if configured
        blk_arr_list_ = curBlkArr[0].m_next;

        delete[] curBlkArr;

        if (available_block_count_ > 0) available_block_count_--;

        if (signo_ != 0) std::raise(signo_);

        return;
      }

      // Release final block and terminate
      delete[] final_block_;
      final_block_ = 0;
      std::terminate();
    }

   private:
    struct Blk {
      Blk* m_next;
    };

    bool init_done_;
    int signo_;
    size_t final_block_size_;
    size_t reserved_block_size_;
    size_t reserved_block_count_;
    unsigned int allocated_block_count_;
    volatile unsigned int available_block_count_;
    Blk* final_block_;
    Blk* blk_arr_list_;
  };
};
}  // namespace simple

#endif  // _INCLUDE_SIMPLE_NEW_HANDLER_H_

Answer 1

My biggest bug bear:

#ifndef _INCLUDE_SIMPLE_NEW_HANDLER_H_
#define _INCLUDE_SIMPLE_NEW_HANDLER_H_

These macros are reserved for the implementation (i.e. you can't use them). An identifier with an initial underscore followed by a capitol letter is reserved for the implementation in any scope.

See: What are the rules about using an underscore in a C++ identifier?

---

What is the purpose of this?

  State s;

  memset(&s, 0, sizeof(s));

If you want to make sure that everything is zeroed in State object then create an appropriate constructor!

---

Not sure what this is for!

  if (!init_done_) {
    return s;
  }

You should design your code so it can't get to this location without it being already initialized correctly. Not sure how you can call a method on an object before it is constructed.

Since you are using a function static variable for all Worker objects creation of this object is already thread safe.

---

Not sure why you would do this:

  s.allocated_block_count = allocated_block_count_;
  s.available_block_count = available_block_count_;

  return s;

Why is this state not already part of the worker object as a State object. Then you can simply return the workers State object!

---

I would have defined the structures like this:

  struct State
  {
    size_t allocated_block_count;
    size_t available_block_count;
  };

  struct FullState {
    bool   init_done;
    int    signo;
    size_t final_block_size;
    bool   final_block_allocated;
    size_t reserved_block_size;
    size_t reserved_block_count;
    State  currentState;
  };

  class Worker
  {
        FullState fullState;
        Blk*      final_block_;
        Blk*      blk_arr_list_;

        State     GetState()     const volatile {return fullState.state;}
        FullState GetFullState() const volatile {return fullState;}
  };

This simplifies a lot of the work you have done.

---

The call to std::set_new_handler(NewHandler::Process); actually returns the current new_handler. You may want to retain this value and use it.

If you fail to allocate any extra memory then you call (if it is not null) the originally available handler (or put it back in place potentially).

---