Что произойдёт после move-касте содержимого shared_ptr?

Question

[MayRiv]

Добрый день! Кто может подробно объяснить происходящее в следующем коде? Я столкнулся в сегфолтом в недрах shared_ptr-а, и подозреваю следующий код в том, что он написан неправильно. Во-первых, я подозреваю, что нельзя кастить в rvalue содержимое shared_ptr, так как это может подменять память, на которую ссылается shared_ptr, и при обнулении счетчика ссылок будет вызван delete случайного куска памяти. Так ли это или я ошибаюсь?

Так же я считаю неправильным сохранение ссылки в классе T2 на объект T1 который мы передаём как rvalue, мне сложно обьяснить что произойдёт на самом деле, но это выглядит неправильно.

Надеюсь, кто-то сможет объяснить как будет работать следующий код, очень хочу разобраться

std::shared_ptr<T1> ptr = std::make_shared(new T1);
   auto  pResultElement = std::make_shared<T2>(std::move(*ptr));

class T2
{
public:
T2(const T1&& var):m_var(std::move(var)){}
private:
T1& var;
}

Comments

[Mercury13]

Попытавшись скрыть личную специфику, вы сделали некомпилирующийся код.
Вот это работает.

#include <iostream>
#include <memory>

using namespace std;

class T1 {};

class T2
{
public:
  T2(T1&& var) : m_var(std::move(var)) {}
private:
  T1&& m_var;
};

int main()
{
    std::shared_ptr<T1> ptr = std::make_shared<T1>();
    auto  pResultElement = std::make_shared<T2>(std::move(*ptr));

    return 0;
}

[Евгений Шатунов]

MayRiv , закинь пожалуйста рабочий код с конкретным указанием ошибки.
По этому коду гадать можно как по кофейной гуще. Толку никакого.

[MayRiv]

Евгений Шатунов,

class PoiSearchPlacesLocation : public tsd::nav::input::mib3::api::types::PlacesLocation
{
public:
   PoiSearchPlacesLocation(const tsd::nav::sdk::poi::api::types::PoiLocation&& poiLocation);
...
private:
   const tsd::nav::sdk::poi::api::types::PoiLocation& m_PoiLocation;
};

Реализация конструктора

PoiSearchPlacesLocation::PoiSearchPlacesLocation(const tsd::nav::sdk::poi::api::types::PoiLocation&& poiLocation)
   : m_PoiLocation(poiLocation){}

И в другой функции

void Mib3PoiSearchTask::notify(std::vector<tsd::nav::sdk::poi::api::types::PoiLocation::SharedPtr> searchResults)
{
   //Convert PoiLocations to PlacesLocations and return result vector
   std::vector<tsd::nav::input::mib3::api::types::PlacesLocation::SharedPtr> resultVector;
   std::vector<tsd::nav::sdk::poi::api::types::PoiLocation::SharedPtr>::const_iterator cit = searchResults.begin();
   const std::vector<tsd::nav::sdk::poi::api::types::PoiLocation::SharedPtr>::const_iterator citEnd = searchResults.end();

   for (; citEnd != cit; ++cit)
   {
      tsd::nav::input::mib3::api::types::PlacesLocation::SharedPtr pResultElement;
      pResultElement = std::make_shared<tsd::nav::input::mib3::poi::search::PoiSearchPlacesLocation>(std::move(**cit));
      resultVector.push_back(pResultElement);
   }
   m_refMib3PoiSearchResultListener.updateSearchResults(resultVector);
}

[Евгений Шатунов]

MayRiv , какая цель у содержания константной ссылки на объект? Какая цель у const rvalue в конструкторе?
По этим вопросам нужно техническое обоснование. Наперед скажу что этот код семантически неверен вне зависимости от наличия обоснования и его качества. Но обоснование все равно нужно. Нужно понять требования к коду и определить направление его дизайна.

Список инициализации приведенного конструктора неясен. Какое поведение от него ожидается?
Какое поведение ожидается от std::make_shared(std::move(...)) в приведенном коде? Какая цель у этой конструкции?

Да, я мог бы сразу указать на все ошибки в этом коде, но не в них суть. В коде видно ошибки другого уровня, поэтому задаю вопросы.

[MayRiv]

Евгений Шатунов, Это не мой код, я согласен в том, он написан неправильно. Не могу обосновать вс ерешния.

Собственно, поэтому мне очень хотелось узнать что именно происходит в данном конкретном случае чтобы распутать краши программы.

[Евгений Шатунов]

MayRiv , мой ответ тут явно не нужен в таком случае, но пояснения я сделаю.
Смотри...
Чисто механически, между ссылкой и указателем разницы мало. Ссылка обязательна к инициализации при объявлении, не может быть переназначена и не требует разыменования. Указатель может быть нулевым, требует разыменования и может быть переназначен. Нужно понимать что и ссылка, и указатель, по своей сути - это граната без чеки в руках. Держи уверенно, но нежно, иначе все будет плохо.

Семантику перемещения нужно правильно понимать. Для этого на хабре есть много статей и всегда есть C++ Core Guideline и стандарт.

const rvalue - это нонсенс. Нужно убрать.
Поле-ссылка на объект может использоваться только если время жизни объекта со ссылкой гарантированно меньше времени жизни объекта по ссылке. Иначе все плохо.
В идеале, раз уж используете разделяемое владение (std::shared_ptr), то его и надо хранить, передавать, разделять и просто использовать.
std::make_shared нужен для того, чтобы сформировать объект с разделяемым владением в едином неразрывном блоке памяти вместе с контрольным счетчиком владения. В качестве аргументов для std::make_shared передаются аргументы конструктора объекта. Зачем разыменовывать один std::shared_ptr и формировать перемещающую ссылку для конструирования другого std::shared_ptr? Это выглядит как нонсенс.
Если надо переместить объект с разделенным владением, то лучше сделать перемещающее присвоение между инстанциями std::shared_ptr.

В целом, это не код, а каша-какаша. Его надо просто переписать.
С дизайном кода в текущих условиях помочь не смогу, т.к. опять же надо дать ответы на те мои вопросы. Нужно сперва понять ожидания от кода и только тогда определять видение и дизайн кода.

[MayRiv]

Спасибо, понимаю

Answer 1

[Mercury13]

После move-каста содержимого shared_ptr мы говорим: объект под shared_ptr идёт на заклание, и в операциях перемещения мы можем потрошить его как угодно — лишь бы объект остался корректным. В данном коде (как я уже писал, он не компилируется) у нас одни ссылки, и не будет ровным счётом ничего: не выполнится ни одна операция перемещения.

Однако возможна такая ситуёвина, никак не связанная с временными ссылками. Во втором есть неуправляемая ссылка на первый — и при определённых условиях она может стать висячей.

Если в T2 всё-таки не ссылка, а экземпляр (T1 m_var), возможно, операция перемещения оставляет объект под ptr в некорректном состоянии, и деструктор ~T1() не может его уничтожить, или какая-то из операций над ptr не подозревает, что объект под ptr выпотрошен.

UPD. Что делает updateSearchResults? Только он может продлить время жизни объекта, сохранив где-то shared_ptr.

UPD2. Константная временная ссылка (const T&&) никакого смысла не несёт (по факту в STL пару раз используется). «Неизменный» и «можно выпотрошить» — это слегка противоречивые вещи.
И всё-таки я грешу на updateSearchResults, который сохраняет где-то объекты. Проблема может решиться, если в PoiSearchPlacesLocation ссылку заменить на shared_ptr. И — ВНИМАНИЕ — соответствующим образом подкорректировать сигнатуру конструктора. У shared_ptr есть конструктор, который берёт под управление любой объект-указатель, создавая новый счётчик, но хреново будет, если один и тот же объект управлять двумя разными счётчиками.

Comments

[MayRiv]

Приложил реальный код.

[Mercury13]

MayRiv, Причина вылета всё ещё не видна (с временем жизни полный порядок). Временная ссылка ни на что не сдалась, ничего она не делает.

[Mercury13]

MayRiv, Какой у вас компилятор? MinGW вообще не позволяет инициализировать обычную ссылку временной.

[MayRiv]

Mercury13, gcc version 6.3.0 20170519 (Ubuntu/Linaro 6.3.0-18ubuntu2~16.04).

Разве полный порядок? Мы же не контролируем время жизни содержимого shared_ptr. Когда оно будет очищенно, обращение по ссылке будет ошибочным, разве нет?

[Mercury13]

MayRiv, Что делает updateSearchResults? Только он может продлить время жизни, сохранив где-то shared_ptr.

[MayRiv]

Ну, там дальше эти shared_ptr-ы отправляются в длинное путешествие по проекту, я предполагаю, что когда-то они всё-таки будут очищены.

[Mercury13]

MayRiv, Так я и думал. Если shared_ptr отправляется в длинное путешествие по проекту и будет жить долгое время после выхода из функции — ссылка в PoiSearchPlacesLocation станет висячей. Замените её на shared_ptr со всеми формальностями, которые shared_ptr окружают.

Если в каком-то другом месте с временем жизни всё в порядке, но совместное владение не нужно или затруднено — в shared_ptr из Си++11 есть конструктор, который позволяет управлять одним объектом, но указывать на другой. В том числе указывать, но не управлять ничем.

Повторяю, временные ссылки тут ничего не делают (не запускается ни одна функция, потенциально способная выпотрошить объект, просто переприсваиваются ссылки), и причина не в них.

[Mercury13]

MayRiv, Когда я говорит, что с временем жизни полный порядок, я имел вот что.
Если функция updateSearchResults не имеет побочного эффекта, то время жизни resultVector — до конца функции. Но если всё же побочный эффект есть — возможна висячая ссылка.

[MayRiv]

Спасибо