When using "new" to initialize unique_ptr<FILE*,File::Close> is the custom deleter responsible for freeing that memory?

Question

Smart Pointers are a new concept for me. I have been trying to wrap a File class around fopen_s and fclose using a smart pointer with a custom deleter (unique_ptr).

Below is my attempt. It successfully compiles, runs, and generates a file named "text.txt" with the contents "Hello World" as expected.

I had to use "new" to initialize unique_ptr in my Open function since make_unique does not appear work with custom deleters. Since I am using "new" is my custom deleter responsible for freeing that allocated memory?

I have stepped through my program (VS2019). File::Close only gets called once. I expected it to be called when "handle" in my File:Open function went out of scope, but this was not the case. This behavior may be influenced by the call to std::move(). Not sure how to investigate further on what happens here.

#include <Windows.h>
#include <memory>
#include <string>
#include <map>

class File
{

private:

//functors - custom deleter
  struct Close { void operator()(FILE** _handle); };

//type definitions
  typedef std::unique_ptr<FILE*,File::Close> Handle;
  typedef std::map<std::string,Handle> HandleMap;

//static members
  static Handle& Open(std::string _name, std::string _mode);
  static HandleMap s_handle_map_;

//variables
  Handle& handle_;
  std::string name_;

public:

//functions
  File(std::string _name, std::string _mode);
  void Write(std::string _message);

};

File::HandleMap File::s_handle_map_;

File::File(std::string _name, std::string _mode)
:handle_(Open(_name,_mode)),
 name_(_name)
{
}

File::Handle& File::Open(std::string _name, std::string _mode)
{
  bool exist = s_handle_map_.count(_name) > 0;

  if (!exist)
  {
    Handle handle(new FILE*(nullptr));

    //open new file
    fopen_s(
      handle.get(),
      _name.c_str(),
      _mode.c_str()
    );

    //transfer ownership of handle
    s_handle_map_.emplace(
      _name,
      std::move(handle)
    );

  }

  return s_handle_map_[_name];
}

void File::Close::operator()(FILE** _handle)
{
  fclose(*_handle);
  *_handle = nullptr;

  //necessary?
  delete _handle;
  _handle = nullptr;
}

void File::Write(std::string _message)
{
  fprintf(*handle_, _message.c_str());
}

int WINAPI WinMain(HINSTANCE _instance, HINSTANCE _previous, LPSTR _cmd, int _show)
{
  File file("test.txt","w");
  file.Write("Hello World\n");
  return 0;
}

Answer 1

Whenever you think of unique_ptr<FILE*, ...>, take a deep breath in, wait a minute, then go on with fstream.

The following code does the same thing, but relies on a proven and well tested C++ standard library. fstream have all the features you expect, including automated closing when they are no longer needed:

int WINAPI WinMain(HINSTANCE _instance, HINSTANCE _previous, LPSTR _cmd, int _show)
{
  fstream file("test.txt", fstream::out);
  file << "Hello World\n";
  return 0;
}  

And you do not need to worry about memory management at all.

Now, generalizing your question :

• if you create a unique_ptr<T,D> yourself based on a new T pointer, the custom deleter D will have the responsibility to delete T. If you don't, you will leak memory (example).
• A better approach would therefore be to keep using the default deleter, and make sure that T's destructor will clean or close anything needed.
• And once you go for a default deleter, the best would be to go for make_unique that has some advantages over the new approach (see here why)

Answer 2

You are making your use of std::unique_ptr more complicated than it needs to be. DO NOT store a FILE** pointer inside the unique_ptr, store a FILE* instead. That is what fopen_s() outputs, and all access to the FILE is done through FILE* not FILE**. You don't need 2 levels of indirection when 1 level will suffice.

Try this:

#include <Windows.h>
#include <memory>
#include <string>
#include <map>

class File
{
private:

//functors - custom deleter
  struct Close { void operator()(FILE* f); };

//type definitions
  typedef std::unique_ptr<FILE,File::Close> Handle;
  typedef std::map<std::string,Handle> HandleMap;

//static members
  static Handle& Open(std::string _name, std::string _mode);
  static HandleMap s_handle_map_;

//variables
  Handle& handle_;
  std::string name_;

public:

//functions
  File(std::string _name, std::string _mode);
  void Write(std::string _message);

};

File::HandleMap File::s_handle_map_;

File::File(std::string _name, std::string _mode)
 : handle_(Open(_name,_mode)), name_(_name)
{
}

File::Handle& File::Open(std::string _name, std::string _mode)
{
  auto iter = s_handle_map_.find(_name);

  if (iter == s_handle_map_.end())
  {
    FILE *f = nullptr;

    //open new file
    if (fopen_s(&f, _name.c_str(), _mode.c_str()) != 0)
        throw std::runtime_error("cannot open file");

    //transfer ownership of handle
    iter = s_handle_map_.emplace(_name, Handle(f)).first;
  }

  return iter->second;
}

void File::Close::operator()(FILE* f)
{
  if (f)
    fclose(f);
}

void File::Write(std::string _message)
{
  fprintf(handle_.get(), "%s", _message.c_str());
}

int WINAPI WinMain(HINSTANCE _instance, HINSTANCE _previous, LPSTR _cmd, int _show)
{
  File file("test.txt", "w");
  file.Write("Hello World\n");
  return 0;
}