Как стать автором
Обновить

Комментарии 17

Рано или поздно мы заметим, что большинство классов-обёрток незначительно отличаются друг от друга: как правило единственное отличие – это функция освобождения ресурса. Подобный подход провоцирует подверженное ошибкам повторное использование кода в стиле «копировать/вставить».

Логично параметризовать эту функцию, зачем копипастить?
С другой стороны, мы видим здесь отличную возможность для обобщения, которая подводит нас к следующему варианту – использованию умных указателей.

Что в имени тебе моем?(с) Пушкин А.С.
Берем сущность которая не является указателем, не имеет ничего с ним общего, и обобщаем до указателя. За что и поплатились:
Причина ошибки в приведённом примере – нарушение концепции NullablePointer типом Handle. Вкратце, модель концепции NullablePointer, должна являться объектом, поддерживающим семантику указателей, и в частности допускающим сравнение с nullptr. Наш Handle, определённый как синоним для int, не является таким объектом. Как следствие, мы не можем использовать std::unique_ptr, для вещей наподобие файловых дескрипторов POSIX или ресурсов OpenGL.

«создаем себе трудности и успешно их преодолеем»
Получили архитектурную ошибку.

Берем сущность которая не является указателем,

Да ладно? А что же это тогда? Число в системной таблице, указывающее на ресурс, не?

Логично параметризовать эту функцию, зачем копипастить?

А вот с этим соглашусь — тоже сразу приходит в голову такая мысль.
Может таблица, может идентификатор узла в списка или дерева.
А может это не идентификатор указывающий на ресурс, а некий объект. Принцип инкапсуляции и слоистая архитектура скрывают от меня эту информацию.
Из подобных примеров у нас есть std::lock_guard, который так же работает в RAII-стиле(забавно, там так же в пример приводят lock_guard).
Был бы сам объект, так бы его и назвали объектом, а не «ручкой» (тут как раз подходит цитата Пушкина). А раз назвали, то для вас это — указатель. Даже если он «указывает» сам в себя (т.е. являться не указателем, а реальным объектом). Вот скажите, url-адрес это объект? А является ли он указателем? Чтобы прочитать статью на хабре, вам url нужен именно как указатель, и как указатель вы его и используете, не заботясь о том, что это объект, у которого можно посчитать длину, выяснить, по https вы пришли или http, и прочую, безусловно важную, но в данном случае, ненужную информацию.

Принцип инкапсуляции в данном случае скрывают от вас, как устроен указатель, но то что это указатель — вам говорят совершенно четко.
В классе присутствуют только конструктор перемещения и оператор перемещения, однако инициализация и присваивание ресурса происходит по значению, что может грозить серьезными ошибками в некоторых случаях, что неприемлемо для «универсальной» обертки.
Пример с std::unique_ptr принципиально некорректен, а спекуляция с использованием std::remove_pointer указывает на полное непонимание принципа его работы. В случае std::unique_ptr работу с ресурсом необходимо оборачивать в работу над указателем на ресурс, а не маскировать ресурс под указатель.

Кстати последняя ревизия Generic Scope Guard and RAII Wrapper — N4189.
Оригинальная статья опубликована в выпуске 126 журнала Overload (апрель 2015).

Очень жаль, что такой некачественный материал публикуется в журналах, можно было бы и проверять материал до публикации. Этим грешат и некоторые книги, а потом эти ошибки из кода примеров оказываются в реальных проектах.
спекуляция с использованием std::remove_pointer указывает на полное непонимание принципа его работы. В случае std::unique_ptr работу с ресурсом необходимо оборачивать в работу над указателем на ресурс, а не маскировать ресурс под указатель.

А можно здесь поподробней? Я использую подобный трюк и хотелось бы узнать в чём-же моё непонимание std::unique_ptr. На примере HANDLE, как бы поступили Вы?
Одно дело что вы используете «трюк» (по сути «хак»), а совсем другое, что он преподносится как единственно верный вариант.
#include <memory>

using Handle = int;
Handle CreateHandle() { Handle h{ -1 }; /*...*/ return h; }
void CloseHandle(Handle& h) { /* ... */ }

struct HandleDeleter {
  void operator()(Handle* h) { CloseHandle(*h);  delete h; }
};
using ScopedHandle = std::unique_ptr<Handle, HandleDeleter>;

int main() {
  ScopedHandle h{ new Handle(CreateHandle()) };
}
Ну, снова таки — из приведённого Вами документа, видно, что автор разбивает ресурсы на «pointer handle type» и «handle types that are not pointers». Для работы с ресурсами первого типа, как раз таки, приводится прекрасный пример использования std::unique_ptr для управления ресурсом (конечно, есть и недостатки).
Пример из N4189
typedef void *HANDLE; // System defined opaque handle type
typedef unsigned long DWORD;
#define INVALID_HANDLE_VALUE reinterpret_cast<HANDLE>(-1)
// Can’t help this, that’s from the OS
// System defined APIs
void CloseHandle(HANDLE hObject);
HANDLE CreateFile(const char *pszFileName,
    DWORD dwDesiredAccess,
    DWORD dwShareMode,
    DWORD dwCreationDisposition,
    DWORD dwFlagsAndAttributes,
    HANDLE hTemplateFile);
bool ReadFile(HANDLE hFile,
    void *pBuffer,
    DWORD nNumberOfBytesToRead,
    DWORD*pNumberOfBytesRead);
// Using std::unique ptr to ensure file handle is closed on scope-exit:
void CoercedExample()
{
    // Initialize hFile ensure it will be ”closed” (regardless of value) on scope-exit
    std::unique_ptr<void, decltype(&CloseHandle)> hFile(
    CreateFile("test.tmp",
        FILE_ALL_ACCESS,
        FILE_SHARE_READ,
        OPEN_EXISTING,
        FILE_ATTRIBUTE_NORMAL,
        nullptr),
    CloseHandle);
    // Read some data using the handle
    std::array<char, 1024> arr = { };
    DWORD dwRead = 0;
    ReadFile(hFile.get(), // Must use std::unique ptr::get()
        &arr[0],
        static_cast<DWORD>(arr.size()),
        &dwRead);
}


Для ресурсов второго типа («handle types that are not pointers») — используется уже новый, предлагаемый API — т.е., я хочу сказать, что простых решений с использованием существующей библиотеки C++ (как в случае с «pointer handle type») — нет.
Так вот Вы же приводите пример, который пытается решить проблему «handle types that are not pointers» c помощью того же std::unique_ptr, и говорите, что использовать std::unique_ptr для «pointer handle type» — спекуляция.
Все так. Спекуляция в том, что проблемы с использованием std::unique_ptr не по назначению выставляются как его недостатки. Очевидно, что unique_ptr не является панацеей, а unique_resource востребован.
В вашем примере происходит лишнее выделение памяти под хендл. При освобождении ресурса функция CloseHandle может бросить исключение и память, выделенная под хендл, не будет освобождена.
В вашем примере происходит лишнее выделение памяти под хендл.

Именно поэтому unique_ptr и не является хорошим инструментом для реализации scope_guard.
При освобождении ресурса функция CloseHandle может бросить исключение и память, выделенная под хендл, не будет освобождена.


Тут у нас есть несколько решений — потребовать условия noexcept(CloseHandle(*h)), просто проигнорировать исключение посредством try-catch, или передать это исключение для дальнейшей обработки:
struct HandleDeleter {
  void operator()(Handle* h) {
  	std::exception_ptr ex;
  	try {
  		CloseHandle(*h); 
  	} catch (...) {
  		ex = std::current_exception();
  	}
	delete h;
  	if (ex){
  		std::rethrow_exception(ex);
  	}
  }
};

Что, впрочем, не является лучшей идеей, учитывая, что deleter будет вызван в деструкторе unique_ptr.
Именно поэтому unique_ptr и не является хорошим инструментом для реализации scope_guard.

Вы сами придумали некорректный пример и теперь приводите его в качестве доказательства бесполезности unique_ptr. Примеры из статьи вполне себе годные
По условиям задачи, CloseHandle бросать исключения не может так как реализована на Си.
По условиям задачи мы пишем удобную обертку для низкоуровневого C-like API, откуда там C++ исключения?

Взаимодействие SEH и исключений в C++ это тема для отдельного разговора. Если я правильно помню, в Winapi используются коды возврата а не SEH чуть менее чем во всех функциях?

Но в целом вы конечно правы)
Во-первых, такие вещи должны быть обязательно обложены тестами, а, во-вторых, в дизайне и коде есть пара моментов. Самое главное в дизайне — ScopedHandle неудачный пример для производной от Вашего Resource.
Дальше коментарии (возможно, тянут на доп статью) пойдут про ScopedHandle, поскольку статья выше в основном об этом.
— я бы сделал ScopedHandle без предложенного Resource, без std::unique_ptr и ограничился бы поддержкой только тривиальных хендлов (static_assert(std::is_trivial<Handle>::value, "The Handle type should be trivial.");). Это существенно всё упрощает.
— заранее неизвестно, что инициализация ресурса в значение по-умолчанию есть правильно, из этого вытекают всякие моменты почти во всех остальных методах
— так же из первого довольно сильно могут усложниться реализации всяких специфичных производных (речь о using ScopedHandle = Resource<struct HandleTag, Handle>, кстати, неплохо было бы добавить final, вряд ли тут будет к месту наследование, и так довольно сложный фундаментальный класс)
— в качестве конструктора по-умолчанию кажется более логично использовать конструктор с одним аргументом, который принимает resource, и значением по-умолчанию
— нет возможности узнать открыт (валидный) handle или нет
— по моему опыту, публичный метод close оказался очень полезен
— иногда надо иметь возможность получить указатель на уже открытый handle (например, чтобы сложить его в какой-нибудь массив), побочный эффект (side effect), очищающий ресурс тут немного некстати. Это действительно очень спорное решение. В зависимости от ситуации тут может лучше вернуть nullptr, если ресурс уже открыт, вернуть указатель на ресурс без изменений, очистить ресурс (как у Вас) или бросить исключение (пока что во всех моих проектах, я был против варианта бросания исключения).
— неплохо было бы иметь Resource& operator=(ResourceType h)
operator const ResourceType&() const noexcept { return resource_; } тут вопрос с константностью, допустим у нас константный объект, тогда мы можем получить ресурс (хендл), и изменить его, так как он скопируется при передаче в функцию. Тут философский вопрос о побитовой и логической константности, но все же упомянуть стоит. Я бы лично сделал неконстантным, больше помощи от компилятора.
— для Resource& operator=(Resource&& other) я бы воспользовался swap idiom, но так тоже хорошо.
— отсутствие operator bool() const и размышлений на эту тему.

В этой связи я бы ввёл какой-нибудь traits, например,
template<typename Handle, Handle InvalidValueT>
struct BaseScopedHandleTraits {
  typedef Handle Handle;
  typedef Handle* HandlePtr;
  // required to be implemented
  //static bool close(Handle h)

  static Handle initialValue() {
    return InvalidValueT;
  }
  static bool test(Handle value) {
    return InvalidValueT != value;
  }
};
template<HANDLE InvalidValue>
struct WindowsHandleTraits : BaseScopedHandleTraits<HANDLE, InvalidValue> {
  static bool close(Handle h) {
    return 0 != CloseHandle(h);
  }
};


Для документации и тем более для статьи обязательно нужно оговорить все моменты закрытия ресурса. Во-первых, состояние после закрытия должно быть рабочим (в Вашем примере неплохо было бы присвоить хендлу спецальное значение), во-вторых, закрытие должно быть строго детерминировано, а, в-третьих, не должны вызываться закрывающие методы от предыдущих ресурсов (у Вас с этим все хорошо на этам дизайна, теги помогают, а мне повезло поработать и с таким кодом).
Плохая идея — изображать из хэндлов указатели, только ради повторного использования unique/shared_ptr. Хотя бы потому, что сигнальные значение указателя — это 0, а сигнальные значение хэндла может быть каким угодно: 0, -1, да хоть 0xDEADFACE. Опять же, «указуемый тип» хэндла не всегда существует. Ну и риск случайно сделать shared_ptr<remove_pointer> с дефолтным делетером вместо специального.
Поэтому для всех будет лучше не впихивать невпихиваемое, а делать отдельные классы умных обёрток.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий