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

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

Чтобы избежать таких ситуаций, есть несколько вариантов решения.


Эм… и где они? Я как-то подсознательно ожидал листинга способов или разбор плюсов и минусов того или иного способа, но вы их даже не предоставили. Как-то даже немного обидно получилось :(
Я представил самый простой вариант. Честно говоря, изначально в статье был ещё вариант с запуском треда и ожиданием на condition_variable, но я не увидел у него преимуществ по сравнению с обычной двухфазной инициализацией и удалил.
Если начать серьезно занудствовать, то мне вариант с двойной инициализацией не нравится хотя бы тем, что нарушает SRP и инкапсуляцию, предоставляю какую-никакую информацию о своей реализации, что не очень архитектурно красиво :)
Edit: впрочем, местами может быть оправдано, например в случае запуска сервера или еще чего-то, где метод start() будет частью интерфейса.
Ну, если занудствовать, то треды вообще слабо вписываются в ООП. ))
НЛО прилетело и опубликовало эту надпись здесь
class Usage {
 public:
  Usage() : th_([this](){ run(); }) {}
  virtual ~Usage() noexcept {}
  virtual void run() {}
 private:
  std::thread th_;
};

Вообще это частный случай более общего антипаттерна с вызовом виртуального метода из конструктора, просто, скажем так, опосредованным образом.
Честно говоря, не очень понял. В общем случае если вызвать виртуальный метод из конструктора, будет просто применено статическое связывание вместо динамического. В данном же случае вызывается именно run() из производного класса, что и приводит к проблемам.
Если я что-то не понял в комментарии, поясните пожалуйста.
Честно говоря, не очень понял. В общем случае если вызвать виртуальный метод из конструктора, будет просто применено статическое связывание вместо динамического.

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

public class Base
{
  public Base()
  {
    System.out.println("Base()");
    Method();
  }

  protected void Method()
  {
    System.out.println("Method() from Base");
  }
}

public class Derived extends Base
{
  private int Field = 10;
  
  public Derived()
  {
    System.out.println("Derived()");
    System.out.println("Field = " + Field);
  }

  @Override
  protected void Method()
  {
    System.out.println("Method() from Derived");
    System.out.println("Field = " + Field);
  }
}

[...]

Derived d = new Derived();

выведет:

Base()
Method() from Derived
Field = 0 < — даже инициализатор при объявлении еще не сработал
Derived()
Field = 10

Уот так уот. Нога простреливается на изичах — «ну как же так, я же инициализировал эту переменную даже не в конструкторе, а аж при объявлении, откуда тут NPE???» От верблюда.

В C#, кстати, было бы примерно то же, но Field в Method() from Derived уже был бы 10. И то радует.

В C# инициализация полей происходит до вызова конструктора.
Поэтому отстрелить ногу можно и там.
Например, вызвав конструктор объекта раньше статического конструктора.

Каким образом вы собираетесь вызвать конструктор объекта раньше статического конструктора?
Static Constructors
A static constructor is called automatically to initialize the class before the first instance is created or any static members are referenced.
    public class Program
    {
        public static void Main(string[] args)
        {
            //Your code goes here
            Console.WriteLine("from main:" + Some.what);
        }

        public class Some{
            public static readonly Some instance = new Some();
            public static readonly string what = "Hello, world!";

            static Some(){
                Console.WriteLine("From static: " + what);
            }

            public Some(){
                Console.WriteLine("From ctor:" + what);
            }
        }
    }

Онлайн компилятор говорит, что конструктор объекта вызывается раньше:


From ctor:
From static: Hello, world!
from main:Hello, world!
Да, действительно, вы правы. Это частный случай, когда экземпляр класса является статическим полем/свойством этого же класса.
Получается, что по приведенной выше ссылке Microsoft себе противоречит, но здесь описывает правильно:
Static Classes and Static Class Members
Static members are initialized before the static member is accessed for the first time and before the static constructor, if there is one, is called.

Двухфазная инициализация в языке с RAII — знатный антипаттерн. (За исключением специальных случаев и весомых причин, само собой.) Означенная проблема с тредом — проблема архитектуры класса. Тред не является частью логики класса и нарушает SRP. Правильнее и чище держать класс отдельно, с интерфейсом пригодным быть использованным в треде. И тестировать проще, чем с тредами возиться в тестах.
Утрированный пример(откуда следует, что низкоуровневыми примитивами лучше вообще не пользоваться в пользу std::async()):


struct A{
 void use(){};
}

...
A a;
std::thread th(&A::use, a); 
do_other_job();
th.join();
НЛО прилетело и опубликовало эту надпись здесь
Кстати, похожие проблемы могут появиться в Java — при работе с потоками в конструкторе.
А именно — если у объекта есть final поля, то гарантируется, что после вызова конструктора их значения будут правильными для всех потоков. Но это неверно, если ссылка на объект была куда-то (например в другой поток) передана внутри конструктора.
У std::thread есть default constructor.
На мой взгляд более простое решение — просто запустить поток в теле конструктора, там уже все members проинициализированы.
И не забыть, что деструктор std::thread позовет terminate, если не сделать join к нему (или detach).
И unique_ptr тоже не нужен, ибо есть operator=, который сделает move.

Примерно так: onlinegdb.com/By5-qJkuN
#include <thread>
#include <string>
#include <iostream>

struct test_t
{
    test_t()
        : str_{"hello world"}
    {
        t_ = std::thread([this]{
            std::cout << "str = " << str_ << std::endl;
        });
        t_.detach();
    }
    
private:
    std::thread t_;
    std::string str_;
};

int main()
{
    test_t t;
    getchar();

    return 0;
}
Если вы собираетесь внутри лямбды, переданной в std::thread, вызывать виртуальные методы из test_t (как делает автор в оригинальном коде), то это не спасет, вы точно так же отстрелите себе ногу по самые яйца.
Не собираюсь.
Комментирую оригинальный посыл статьи, про рождение тредов из initialization list конструктора.

Тема вызова виртуальных функций из конструктора — не связана с «как отстрелить себе ногу с std::thread».
Особенно прекрасно будет если из конструктора базового класса вылетит исключение, а потом вызовется run() у уже удаленного ( а точней даже и не созданного ) объекта.

Полагаю, что разделение runnable объектов и самих тредов, частично решит проблему. Т.е. в схеме BaseRunnable::run() -> DervedRunnable::run(), а потом std::thread(&Derived::run, derived) не должно быть проблем.

Насколько я понял, это просто фундаментальные ошибки. Если убрать треды из обоих верхних примеров, то проблема никуда не уйдёт.
Вызывать функцию класса из списка инициализации для поля можно только такую, которая использует лишь поля класса, что были уже инициализированы (и стоят выше этого поля).
Вызывать виртуальные функции из конструктора и деструктора вообще нельзя (вызов ведёт на ещё не построенный или уже разрушенный «верхний этаж»).
Вызывать виртуальные функции из конструктора и деструктора вообще нельзя (вызов ведёт на ещё не построенный или уже разрушенный «верхний этаж»).

Не ведёт, т.к. применяется статическое связывание вместо динамического. Это уже обсуждалось в комментариях выше.
Хотя, в целом скорее соглашусь — проблема не завязана только на треды. Я лишь описываю достаточно типичный случай, когда с этой проблемой можно столкнуться, используя треды.
Посмотрел стандарт — хм, да, действительно, не ведёт. Согласен. Спасибо, буду знать.
Как по мне все проблемы исчезают, если мы будем использовать функтор вместо класса с run методом.
#include <thread>

class Runner
{
public:
	void operator()()
	{
		// Do some staff
	}
};

class ScopedThread
{
public:
	ScopedThread(std::thread&& aThread)
		: myThread(std::move(aThread))
	{

	}

	~ScopedThread()
	{
		if (myThread.joinable())
			myThread.join();
	}
private:
	std::thread myThread;
};

void main()
{
	Runner()(); // Runs in current thread
	ScopedThread(std::thread{ Runner()}); // Runs in separate thread 
}

Согласен. Кстати, в Qt так и рекомендуют делать с их потоками. Там чуть другое апи, но суть та же.

в 2019-м году реализовывать свой thread класс с виртуальным методом run — моветон неслыханный. Вместо наследования от такого AbstractThread и переопределения run можно просто создать std::thread, параметризованный лямбдой.
Так я и не рекламирую такой способ. Конечно, при написании нового кода есть варианты получше. ;)
Зарегистрируйтесь на Хабре , чтобы оставить комментарий

Публикации

Истории