​​Зачем локать мьютексы в разном порядке
#новичкам

Можете подумать, что раз всем так известно, что мьютексы опасно лочить в разном порядке, то почему вообще кому-то в голову может прийти наступить на грабли и накалякать такое своими программисткими пальчиками снова? Просто не пишите хреновый код и будет вам счастье.

Дело в том, что иногда это не совсем очевидно. Точнее почти всегда это не очевидно.

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

Разберем чуть более сложный пример:

struct SomeSharedResource {
  void swap(SomeSharedResource& obj) {
    {
      std::lock_guard lg{mtx};
      // just for results reproducing
      std::this_thread::sleep_for(std::chrono::milliseconds(10));
      std::lock_guard lg1{obj.mtx};
      // handle swap
    }
  }
  std::mutex mtx;
};

int main() {
  SomeSharedResource resource1;
  SomeSharedResource resource2;  
  std::mutex m2;
  std::thread t1([&resource1, &resource2] {
    resource1.swap(resource2);
    std::cout << "1 Do some work" << std::endl;
  });
  std::thread t2([&resource1, &resource2] {
    resource2.swap(resource1);
    std::cout << "2 Do some work" << std::endl;
  });

  t1.join();  
  t2.join();
}

Все просто. У нас есть пара объектов, которые используются в качестве разделяемых ресурсов (их могут изменять более 1 потока). Объекты могут обмениваться между собой данными.

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

И вроде снаружи кажется, что мы всегда лочим в одном порядке. Пока мы не будем обменивать данные одних и тех же объектов в разных потоках в разном порядке. Тогда первый поток может залочить мьютекс первого объекта и, допустим, заснуть. А второй поток первым залочит мьютекс второго объекта. И все. Приплыли.

И это все еще может показаться детским садом и очевидушкой. Только вот сам момент взятия замка уже не выглядит так подозрительно. А сами объекты могут лежать в каких-то страшных структурах данных и могут быть разбросаны по коду. В таком случае все не так очевидно становится.

В следующий раз поговорим, как же не допускать такие ошибки.

Don't be obvious. Stay cool.

#concurrency

​​Локаем много мьютексов
#опытным

Cтандартное решение этой проблемы дедлока из постов выше - лочить замки в одном и том же порядке во всех потоках. Но как это сделать? Они не же на физре, "по порядку рассчитайсьььь" не делали.

Можно конечно на ифах городить свой порядок на основе, например, адресов мьютексов. Но это какие-то костыли и так делать не надо.

Так как проблема довольно стандартная, то и решение мы скорее всего найдем в том же стандарте.

std::scoped_lock был введен в С++17 и представляет собой RAII обертку над локом множества мьютексов. Можно сказать, что это std::lock_guard на максималках. То есть буквально, это обертка, которая лочит любое количество мьютексов от 0 до "сами проверьте верхнюю границу".

Но есть один важный нюанс. Никак не гарантируется порядок, в котором будут блокироваться замки. Гарантируется лишь то, что выбранный порядок не будет приводить к dead-lock'у.

Пример из прошлого поста может выглядеть теперь вот так:

struct SomeSharedResource {
  void swap(SomeSharedResource& obj) {
    {
      std::scoped_lock lg{mtx, obj.mtx};
      // handle swap
    }
  }
  std::mutex mtx;
};

int main() {
  SomeSharedResource resource1;
  SomeSharedResource resource2;  
  std::mutex m2;
  std::thread t1([&resource1, &resource2] {
    resource1.swap(resource2);
    std::cout << "1 Do some work" << std::endl;
  });
  std::thread t2([&resource1, &resource2] {
    resource2.swap(resource1);
    std::cout << "2 Do some work" << std::endl;
  });

  t1.join();  
  t2.join();
}

И все. И никакого дедлока.

Однако немногое лишь знают, что std::scoped_lock - это не только RAII-обертка. Это еще и более удобная обертка над "старой" функцией из С++11 std::lock.

О ней мы поговорим в следующий раз. Ведь не всем доступны самые современные стандарты. Да и легаси код всегда есть.

Be comfortable to work with. Stay cool.

#cpp17 #cpp11 #concurrency

std::lock
#опытным

Сейчас уже более менее опытные разрабы знают про std::scoped_lock и как он решает проблему блокировки множества мьютексов. Однако и в более старом стандарте С++11 есть средство, позволяющее решать ту же самую проблему. Более того std::scoped_lock - это всего лишь более удобная обертка над этим средством. Итак, std::lock.

Эта функция блокирует 2 и больше объектов, чьи типы удовлетворяют требованию Locable. То есть для них определены методы lock(), try_lock() и unlock() с соответствующей семантикой.

Причем порядок, в котором блокируются объекты - не определен. В стандарте сказано, что объекты блокируются с помощью неопределенной серии вызовов методов lock(), try_lock и unlock(). Однако гарантируется, что эта серия вызовов не может привести к дедлоку. Собстна, для этого все и затевалось.

Штука эта полезная, но не очень удобная. Сами посудите. Эта функция просто блокирует объекты, но не отпускает их. И это в эпоху RAII. Ай-ай-ай.

Поэтому ее безопасное использование выглядит несколько вычурно:

struct SomeSharedResource {
  void swap(SomeSharedResource& obj) {
    {
      // !!!
          std::unique_lock<std::mutex> lk_c1(mtx, std::defer_lock);
          std::unique_lock<std::mutex> lk_c2(obj.mtx, std::defer_lock);
          std::lock(mtx, obj.mtx);
      // handle swap
    }
  }
  std::mutex mtx;
};

int main() {
  SomeSharedResource resource1;
  SomeSharedResource resource2;  
  std::mutex m2;
  std::thread t1([&resource1, &resource2] {
    resource1.swap(resource2);
    std::cout << "1 Do some work" << std::endl;
  });
  std::thread t2([&resource1, &resource2] {
    resource2.swap(resource1);
    std::cout << "2 Do some work" << std::endl;
  });

  t1.join();  
  t2.join();
}

Раз мы все-таки за безопасность и полезные практики, то нам приходится использовать std::unique_lock'и на мьютексах. Только нужно передать туда параметр std::defer_lock, который говорит, что не нужно локать замки при создании unique_lock'а, его залочит кто-то другой. Тем самым мы убиваем 2-х зайцев: и RAII используем для автоматического освобождения мьютексов, и перекладываем ответственность за блокировку замков на std::lock.

Можно использовать и более простую обертку, типа std::lock_guard:

struct SomeSharedResource {
  void swap(SomeSharedResource& obj) {
    {
      // !!!
          std::lock(mtx, obj.mtx);
          std::lock_guard<std::mutex> lk_c1(mtx, std::adopt_lock);
          std::lock_guard<std::mutex> lk_c2(obj.mtx, std::adopt_lock);
      // handle swap
    }
  }
  std::mutex mtx;
};

Здесь мы тоже используем непопулярный конструктор std::lock_guard: передаем в него параметр std::adopt_lock, который говорит о том, что мьютекс уже захвачен и его не нужно локать в конструкторе lock_guard.

Можно и ручками вызвать .unlock() у каждого замка, но это не по-православному.

Использование unique_lock может быть оправдано соседством с условной переменной, но если вам доступен C++17, то естественно лучше использовать std::scoped_lock.

Use modern things. Stay cool.

#cpp11 #cpp17 #concurrency

Спасибо всем, кто поделился своими лав-стори в комментах.
Такие разные истории, но в то же время одинаковые.
И во всех них сквозит одна мысль: "Двигайся по зову своего внутреннего огня. Он приведет тебя к цели"
Если что-то не нравится, никогда не поздно это поменять. Если что-то нравится, делай это и жизнь будет радостней. И это касается не только программирования естественно.

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

Be passionate. Stay cool.

Порядок взятия замков. Ч1
#опытным

В этом посте я намеренно совершил одну ошибку, как байт на комменты и следующие посты. Но вы как-то пропустили ее, хотя и разбирали ту же самую тему в комментариях.

Cтандартное решение этой проблемы дедлока из постов выше - лочить замки в одном и том же порядке во всех потоках.

На самом деле залочивание мьютексов в одном и том же порядке - не общепринятая концепция решения проблемы блокировки множества замков. Это лишь одна из стратегий. И она не используется в стандартной библиотеке!

Когда-то у меня тоже была уверенность, что std::scoped_lock блочит мьютексы в порядоке их адресов. Условно, в начале лочим замок с меньшим адресом. Потом с большим и так далее.

Но как я и написал в середине того же поста, что std::scoped_lock вообще не гарантирует никакого порядка залочивания. Гарантируется только что такой порядок не может привести к дедлоку.

Давайте посмотрим на следующий пример:

std::mutex log_mutex;

struct MyLock {
  MyLock() = default;
  
  void lock() {
    mtx.lock();
    std::lock_guard lg{log_mutex};
    std::cout << "Lock at address " << &mtx << " is acquired." << std::endl;
  }

  bool try_lock() {
    auto result = mtx.try_lock();
    std::lock_guard lg{log_mutex};
    std::cout << std::this_thread::get_id() << " Try lock at address " << &mtx << ". " << (result ? "Success" : "Failed") << std::endl;
    return result;
  }
  
  void unlock() {
    mtx.unlock();
    std::lock_guard lg{log_mutex};
    std::cout << "Lock at address " << &mtx << " is released." << std::endl;
  }

private:
std::mutex mtx;
};

MyLock lock1;
MyLock lock2;
MyLock lock3;  

constexpr size_t iteration_count = 100;

void func_thread1() {
  size_t i = 0;
  while(i++ < iteration_count) {
    {
      std::lock_guard lg{log_mutex};
      std::cout << std::endl << std::this_thread::get_id() << " Start acquiring thread1" << std::endl << std::endl;
    }
    std::scoped_lock scl{lock1, lock2, lock3};
    std::lock_guard lg{log_mutex};
    std::cout << std::endl << std::this_thread::get_id() << " End acquiring thread1" << std::endl << std::endl;
  }
}

void func_thread2() {
  size_t i = 0;
  while(i++ < iteration_count) {  
    {
      std::lock_guard lg{log_mutex};
      std::cout << std::endl << std::this_thread::get_id() << " Start acquiring thread2" << std::endl << std::endl;
    }  
    std::scoped_lock scl{lock3, lock2, lock1};
    std::lock_guard lg{log_mutex};
    std::cout << std::endl << std::this_thread::get_id() << " End acquiring thread2" << std::endl << std::endl;
  }
}

int main() {
  std::jthread thr1{func_thread1};
  std::jthread thr2{func_thread2};
}

Все довольно просто. Определяем класс-обертку вокруг std::mutex, который позволит нам логировать все операции с ним, указывая идентификатор потока. Определяем все методы, включая try_lock, чтобы MyLock можно было использовать с std::scoped_lock.

Также определяем 2 функции, которые будут запускаться в разных потоках и пытаться локнуть сразу 3 замка. И блокируют они их в разных порядках. Все это в циклах, чтобы какую-то статистику иметь. С потоками сложно детерминировано общаться.

Запускаем это дело и смотрим на вывод консоли. Там будет огромное полотно текста, но вы сможете заметить в нем "несостыковочку" с теорией про блокировку по адресам. Возможный кусочек вывода:

129777453237824 Start acquiring thread1

129777453237824 Lock at address 0x595886d571e0 is acquired.
129777453237824 Try lock at address 0x595886d57220
129777453237824 Try lock at address 0x595886d57260
...
129777442752064 Start acquiring thread2

129777442752064 Lock at address 0x595886d57260 is acquired.
129777442752064 Try lock at address 0x595886d57220
129777442752064 Try lock at address 0x595886d571e0

Тут наглядно показано, что мьютексы лочатся в противоположном порядке в разных потоках. И в коде мьютексы передаются в скоупд лок в противоположном порядке. А значит дело тут не в адресах, а в чем-то другом. О этом в следующий раз.

Don't get fooled. Stay cool.

#concurrency #cpp17

​​Порядок взятия замков. Ч2
#опытным

Так в каком же порядке блокируются мьютексы в std::scoped_lock? Как я уже и говорил - в неопределенном. Но и здесь можно немного раскрыть детали.

The objects are locked by an unspecified series of calls to lock, try_lock, and unlock.

Mutex-like объекты блочатся недетерминированной серией вызовов методов lock(), unlock() и try_lock().

Алгоритм можно представить некой игрой в поддавки. Мы пытаемся поочереди захватить мьютексы. И если на каком-то из какой-то из них занят, то мы не ждем, пока он освободится. Мы освобождаем все свои мьютексы, давая возможность другим потокам их захватить, и после этого начинаем пытаться захватывать замки заново.

Зачем так сложно?

А просто физически не может произойти ситуации, когда два потока захватили по набору замков и ждут, пока другие освободятся(а это и есть дедлок). Один из потоков точно пожертвует захваченными ресурсами в пользу другого и исполнение продолжится.

При запуске кода из предыдущего поста вы можете увидеть вот такую картину(но не гарантирую):

128616222426688 Lock at address 0x56aef94a31e0 is acquired.
128616222426688 Try lock at address 0x56aef94a3220. Success
128616222426688 Try lock at address 0x56aef94a3260. Failed
128616222426688 Lock at address 0x56aef94a3220 is released.
128616222426688 Lock at address 0x56aef94a31e0 is released.
128616222426688 Lock at address 0x56aef94a31e0 is acquired.
128616222426688 Try lock at address 0x56aef94a3220. Success
128616222426688 Try lock at address 0x56aef94a3260. Failed
128616222426688 Lock at address 0x56aef94a3220 is released.
128616211940928 Lock at address 0x56aef94a3260 is acquired.
128616211940928 Try lock at address 0x56aef94a3220. Success
128616211940928 Try lock at address 0x56aef94a31e0. Success

Надо понимать, что это многопоточка и каких-то упорядоченных логов между потоками быть не может, поэтому надо немного напрячь извилины.

(0x56aef94a31e0 - первый мьютекс, 0x56aef94a3220 - второй, 0x56aef94a3260 - третий)

Смотрим. Поток 128616222426688 локает первый замок, пытается локнуть второй и делает это успешно, а вот третий не получается. Значит он освобождает свои два и пытается начать заново. Дальше видим такую же картину - на третьем мьютексе try_lock прошел неудачно -> освобождаем имеющиеся.

Тут просыпается второй поток 128616211940928. И пишет, что он сразу заполучил третий замок. На самом деле он заблочил его еще до начала этой ситуации, так как первый поток не мог залочить третий мьютекс. Просто поток 128616211940928 уснул между локом и выводом на консоль. И дальше пытается захватить второй и первый замки и у него это успешно получается.

То есть поток 128616222426688 пожертвовал своими захваченными замками в пользу потока 128616211940928.

Вот так выглядит реализация функции std::lock(которая лежит под капотом std::scoped_lock) в gcc:

template<typename _L1, typename _L2, typename... _L3>
void lock(_L1& __l1, _L2& __l2, _L3&... __l3)
{
#if __cplusplus >= 201703L
  if constexpr (is_same_v<_L1, _L2> && (is_same_v<_L1, _L3> && ...))
  {
    constexpr int _Np = 2 + sizeof...(_L3);
    unique_lock<_L1> __locks[] = {
      {__l1, defer_lock}, {__l2, defer_lock}, {__l3, defer_lock}...
    };
    int __first = 0;
    do {
      __locks[__first].lock();
      for (int __j = 1; __j < _Np; ++__j)
      {
        const int __idx = (__first + __j) % _Np;
        if (!__locks[__idx].try_lock())
        {
          for (int __k = __j; __k != 0; --__k)
          __locks[(__first + __k - 1) % _Np].unlock();
          __first = __idx;
          break;
        }
      }
    } while (!__locks[__first].owns_lock()); 
    for (auto& __l : __locks)
      __l.release();
  }
  else
#endif
  {
    int __i = 0;
    __detail::__lock_impl(__i, 0, __l1, __l2, __l3...);
  }
}

Кто сможет - разберется, но что тут происходит в сущности - я описал выше.

Give something up to get something else. Stay cool.

#concurrency #cpp17

Почему не используют стратегию блокировки по адресам?
#опытным

Точного ответа от разработчиков стандартной библиотеки мы не услышим, но я приведу некоторые рассуждения, которые могут натолкнуть на некоторые мысли.

Начнем с того, что локать один мьютекс - это норма. Все так делают, никто от этого не помер.

Проблемы и эти ваши дедлоки начинаются только тогда, когда поток по какой-то причине блокируется с уже захваченным локом.

А именно это и происходит при вызове метода lock(). Поток мытается захватить мьютекс и если не получается - блокируется до того момента, пока мьютекс не освободится.

Поэтому любая схема с последовательным вызовом методов lock() будет подвержена дедлокам.

А в схеме с упорядоченным по адресам блокировкой именно так и происходит.

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

А что если в одном потоке замки будут лочиться через scoped_lock по адресной схеме, а в другом потоке - одиночно?

            1 поток            |            2 поток              |
-------------------------------|---------------------------------|
 lock(mutex2) // УСПЕШНО       |                                 |
                               | scoped_lock()                   |
                               |    lock(mutex1) // УСПЕШНО      |
                               |    lock(mutex2) // ОЖИДАНИЕ ... |
 lock(mutex1) // ОЖИДАНИЕ...   |                                 |

В этом случае настанет дедлок. Спасибо за пример Сергею Борисову.

Ну или другую ситуацию рассмотрим: есть 4 замка l1, l2, l3, l4. Поток захватил замок с самым большим адресом l4 и надолго(потенциально навсегда) заблокировался.
Но другие треды продолжают нормально работать. И они иногда захватывают пары мьютексов. Все продолжается нормально, пока один из потоков не пытается залочить l3 и l4. Из-за ордеринга захватится l3, а дальше поток будет ждать освобождения l4 aka заблокируется. Дальше другой поток будет пытаться захватить l2 и l3. Он захватит l2 и будет дожидаться l3.

Логику можно продолжать и дальше. Таким образом из-за одного мьютекса и немного поломанного потока может остановиться вся программа.

std::mutex l1, l2, l3, l4;
// Пусть я как-то гарантирую, что они пронумерованы в порядке
 возрастания адресов и std::scoped_lock(sc_lock для краткости) 
работает с помощью сортировки по адресам

1 поток      |     2 поток    |     3 поток    |       4 поток       
-------------|----------------|----------------|----------------
 l4.lock();  |                |                |
//blocks here|                |                |
             |sc_lock(l3, l4);|                |
             | // lock l3     |                |
             | // blocks on l4|                |
                              |sc_lock(l2, l3);|
                              | // lock l2     |
                              | // blocks on l3|
                              |                | sc_lock(l1, l2);
                                               | // lock l1      
                                               | // blocks on l2

Примеры немного преувеличены, но тем не менее они говорят о том, что схема с адресами не совсем безопасна.

Так может тогда вообще не будем блокироваться при уже захваченном мьютексе? Именно это и делают в реализации стандартной библиотеки. Первый захват мьютекса происходит через обычный lock(), а остальные мьютексы пытаются заблокировать через try_lock. Можно сказать, что это lock-free взятие замка. Если мьютекс можно захватить - захватываем, если нет, то не блокируемся и дальше продолжаем исполнение. Так вот в случае, если хотя бы один try_lock для оставшихся замков вернул false, то реализация освобождает все захваченные замки и начинает попытку снова.

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

ПРОДОЛЖЕНИЕ В КОММЕНТАРИЯХ

#concurrency

​​Еще один способ залочить много мьютексов

Последний пост из серии.

Вот у мьютекса есть метод lock, который его захватывает. А разработчики stdlib взяли и сделали функцию std::lock, которая лочит сразу несколько замков.

Также у мьютекса есть метод try_lock, который пытается в неблокирующем режиме его захватить. "Авось да получится". И видимо по аналогии с lock в стандартной библиотеке существует свободная функция std::try_lock, которая пытается захватить несколько мьютексов так же в неблокирующем режиме.

template< class Lockable1, class Lockable2, class... LockableN >  
int try_lock( Lockable1& lock1, Lockable2& lock2, LockableN&... lockn );

То есть в этой функции банально в цикле на каждом из переданных аргументов вызывается try_lock и, если какой-то из вызовов завершился неудачно, то все занятые замки освобождаются и возвращается индекс мьютекса, на котором выполнение зафейлилось. Если все завершилось успешно aka все замки захвачены, возвращается -1.

Чтобы с помощью std::try_lock наверняка захватить все замки, нужно крутиться в горячем цикле и постоянно вызывать std::try_lock, пока она не вернет -1.

Непонятно, зачем эта функция нужна в прикладном программировании при наличии std::scoped_lock и std::lock, которые еще и удобно оборачивают все эти циклы, скрывая все эти кишки в деталях реализации.

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

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

Stay useful. Stay cool.

#concurrency

​​Могу ли я вызвать функцию main?
#опытным

Вопрос из разряда "а что если" и особо практического смысла не имеет. Но когда это нас останавливало? Без знаний стандарта на этот вопрос вряд ли можно ответить правильно, но я попробую хотя бы приблизится к этому.

Прородителем плюсов был язык С, поэтому давайте сначала посмотрим, что там творится по этому поводу.

В C нет запрета вызывать main(). А все, что не запрещено - разрешено. Вот и вы легко можете вызвать main() из любого места программы.

Можно даже рекурсивно вызвать главную функцию и из этого можно придумать что-то более менее рабочее. Например:

#include <stdio.h>
int main (int argc, char *argv[]) {
    printf ("Running main with argc = %d, last = '%s'\n",
        argc, argv[argc-1]);
    if (argc > 1)
        return main(argc - 1, argv);
    return 0;
}

Если это запускать, как 'recursive_main 1 2 3', то вывод будет такой:

Running main with argc = 4, last = '3'
Running main with argc = 3, last = '2'
Running main with argc = 2, last = '1'
Running main with argc = 1, last = './recursive_main'

Но вы скорее попадете на переполнение стека от неконтролируемой рекурсии, чем сделаете что-то полезное.

Ну и конечно, в С сложно сделать так, чтобы код выполнялся в глобальной области.

А вот в С++ это сделать суперпросто. В конструкторах глобальных объектов. И вот здесь уже интересно. Инициализация глобальных переменных имеет свой определенный порядок. Что будет, если мы в этот порядок вклинимся и запустим программу раньше?

Такое чувство, что ничего хорошего мы не получим.

В случае с рекурсией любой код теоретически может уйти в переполнение, поэтому как будто и не особо важно, main это или нет. Не запрещать же рекурсию из-за возможности ее никогда не остановить.

Но вот если мы можем прервать подготовку программы к вызову main ее преждевременным вызовом - у нас 100% возникнут проблемы.

Скорее всего это одна из мажорных причин, почему в С++ нельзя вызывать main никаким образом. Если происходит обратное, то программа считается ill-formed.

Компиляторы по идее должны тут же прервать компиляцию при упоминании main в коде. Но вы же знаете эти компиляторы. Слишком много им свободы дали. Скорее всего, вы сможете скомпилировать свой код с вызовом main и все заработает. Только если прописать какой-нибудь --pedantic флаг, то вам скажут "атата, так делать низя".

В общем, не думаю, что у вас было желание когда-то вызвать main. Однако сейчас вы точно знаете, что так делать нельзя)

Follow the rules. Stay cool.

#cppcore #goodoldc

Ревью

Сегодня вам на суд выносится довольно простой, может даже игрушечный, кусок многопоточного кода, чтобы даже начинающие смогли высказать, чем они в нем недовольны. Однако там несколько не самых очевидных подводных камней, так что будьте внимательны.

Для новичков, кто еще не видел эту рубрику. В рубрике #ревью мы выкладываем сюда отрывок кода, а вы в комментах по существу говорите, что в этом коде не так и как бы вы это исправили. Как бы вы это делали на реальном ревью в своем проекте. Того, кто найдет больше всего недостатков, я завтра упомяну в итоговом посте-компиляции из всех найденных проблем.

Вот собственно код:

struct Task {
  void Execute() {
    // pretend that this is very long calculations
    std::this_thread::sleep_for(std::chrono::seconds(2));
  }
}; 

void WorkingThread(std::deque<Task> queue) {
  std::mutex mtx;
  mtx.lock();
  while (!queue.empty()) {
    auto elem = queue.front();
    queue.pop_front();
    elem.Execute();
    lck.unlock();
    // to get other threads opportunity to work
    std::this_thread::sleep_for(std::chrono::milliseconds(1));
    lck.lock();
  }
}

int main() {
  std::deque<Task> queue(10);
  std::thread thr1{WorkingThread, queue}, thr2{WorkingThread, queue};
}

Чего ждем? Айда в комменты поливать г ревьюить код.

Ask for objective critique. Stay cool.

​​Результаты ревью

Всем участникам спасибо за развернутые ответы. Самый объемный комментарий с большим количеством правильных предположений относительно кода написал Alex D. Давайте ему все вместе похлопаем👏👏👏.

Также удивило, что некоторые предоставили прям свои исправленные версии кода. Спасибо большое за вовлеченность😌. Ваши примеры помогают новичкам по-другому посмотреть на задачу и увидеть альтернативные и более корректные версии ее решения.

Но мы не будем уж погружаться совсем далеко в дебри и не будет совсем переписывать кусок, чтобы в нем появлялись атомики, тредпулы, кондвары и бечконечный разбор очереди до graceful shutdown. Уже в одном из прошлых ревью разбирали, как может выглядеть более совершенная версия воркера для очереди. Все ваши комменты и примеры кода с более продвинутой обработкой в любом случае имеют смысл. Просто этот пример довольно учебный и планировал показывать вполне конкретные вещи, не претендуя на что-то сложное.

Но и на этом поле есть, где развернуться.

Итак, пойдем по в рандомном порядке:

💥 Очередь копируется в воркер потока, поэтому мы обрабатываем не просто копию очереди, а копии!. Каждый поток имеет свою копию. Передача по значению не была бы прям уж сильной проблемой, если бы был один воркер. А их тут 2. И вся работа будет делаться дважды. Это конечно не порядок. Меняем параметр функции на ссылку.

💥 Но если вы думаете, что вы так передадите потоку ссылку - вы ошибаетесь! На самом деле для всех объектов-аргументов потоковой функции во внутреннем сторадже нового потока создается их копия, чтобы не иметь дело с висячими ссылками. Если вы хотите передать истинную ссылку в поток, то надо воспользоваться std::ref.

💥 Нет join'ов у потоков. Надо их либо вызвать, либо использовать jthread из С++20.

💥 Одна из самых больших непоняток в коде для комментаторов - что за переменная lck? На самом деле автор кода немного с ним экспериментировал и просто допустил ошибку, не переименовав lck в mtx. Такое бывает, особенно с новичками. Особенно в проектах без CI.

💥 Если вы используете мьютекс, это не значит, что все автоматически становится потокобезопасным. В этом примере мьютекс никак не защищает критическую секцию! А как он может защитить, если он является локальной переменной. То есть каждый поток будет иметь свою копию этого мьютекса. Не порядок. Его надо сделать либо глобальным вообще, либо статической локальной переменной.

💥 Использование чистых вызовов lock и unlock на мьютексах является очень плохой практикой. В случае изначально пустой очереди или такого распределения расписания потоков, что очередь окажется пуста до входа в while, один из потоков никогда не отпустит лок.
Да и вот это засыпание после execute выглядит очень криво.
Нужно использовать RAII обертки, типа std::lock_guard.

💥 Но в нашем случае lock_guard будет плохим решением. Во-первых, не очень понятно, куда его вставлять. Изначальная проверка на пустоту тоже должна быть "обезопашена". Тогда надо ставить до while. Но в этом случае один из потоков будет выполнять всю работу целиком. А нам хотелось бы использовать преимущества многопоточки.

И еще один нюанс: у нас под локом выполняется довольно тяжелая функция, которая долго выполняется. И все это время другие потоки будут простаивать без дела. А если у нас со временем их станет больше? Еще больше простоя. А время - это деньги.

В общем, нехватает гибкости. Хочется иметь возможность отпускать замок на время, а потом опять его захватывать. Но и преимуществами RAII тоже хочется пользоваться.
Выход - std::unique_lock. Он позволяет делать и то, и то.

💥 Так как выполнение задачи теперь не под локом, то sleep в воркере не нужен, другие потоки имеют достаточно времени, чтобы захватить мьютекс.

💥 Неплохо бы обернуть выполнение задачи в блок try-catch. Так если вылетит исключения из выполнения одной задачи, мы можем как-то обработать эту ситуацию и пойти работать дальше, а не просто свалиться в std::terminate.
Пусть этот пример останется "учебным", но теперь хотя бы корректным.

ПРОДОЛЖЕНИЕ В КОММЕНТАРИЯХ

Fix your flaws. Stay cool.

​​Инкапсуляция и структуры
#новичкам

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

Однако, как и со многими такими догматами случается, не всегда нужно следовать этой концепции. Давайте посмотрим на следующий код:

class MyClass
{
    int m_foo;
    int m_bar;
public:
    int addAll() const;
    int getFoo() const;
    void setFoo(int foo);
    int getBar() const;
    void setBar(int bar);
};
int MyClass::addAll() const
{
    return m_foo + m_bar;
}

int MyClass::getFoo() const
{
    return m_foo;
}

void MyClass::setFoo(int foo)
{
    m_foo = foo;
}

int MyClass::getBar() const
{
    return m_bar;
}

void MyClass::setBar(int bar)
{
    m_bar = bar;
}

Выглядит солидно. Сеттеры, геттеры, все дела. Но вот души в этом коде нет. Он какой-то.. Бесполезный чтоли.

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

Этим грешат новички: поначитаются модных концепций(или не очень модных) и давай скрывать все члены.

Brah... Don't do this...

Здесь нет никаких инвариантов, которые нужно было бы сохранять. Интерфейс класса и так позволяет все, шо хош делать с объектом.

Если ваш класс используется просто как хранилище данных, то это ближе не к классу, а к сишной структуре. Сделайте вы уже поля открытыми и замените ключевое слово class на более подходящее struct.

struct MyClass
{
    int m_foo;
    int m_bar;
}

Делов-то. Зато прошлым примером можно хорошо отчитываться за количество написанных строчек кода=)

Show your inner world to others. Stay cool.

#cppcore #goodpractice #OOP

Return в main

На собеседовании тут встретил сеньора-помидора, который говорил, что код не скомпилируется, если не написать return 0 в конце. Думал, что все уже знают эту особенность, но раз нет - придется рассказать. Правда это довольно коротко будет.

В С++ разрешено не указывать возвращаемое значение для функции main. Но только для нее! Это единственное исключение.

Однако процессу требуется какой-то код возврата по завершению работы, это требование ОС. Поэтому, если исполнение дошло до конца main и ничего плохого не произошло - подразумевается, что возвращается 0. Компилятор также добавляет соответствующую инструкцию в асм.

На счет сишки точно не знаю, но вроде в С99 можно было опускать инструкцию возврата.

#include <iostream>

int main() {
    std::cout << "Hello, World!" << std::endl;
}

Поэтому этот код абсолютно валидный и не противоречит стандарту.

Так что все в порядке, город может спать спокойно без return.

Sleep well. Stay cool.

#cppcore

​​Поля_класса
#опытным

Очень часто в компаниях прибегают к различного рода код-стайлам для обозначения полей класса и выделения из на фоне серой массы локальных переменных.

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

Удобно еще бывает различать открытые и закрытые поля с помощью наличия или отсутствия подчеркивания.

Чисто идейно мне нравится такой стайл: открытые члены без подчеркиваний, защищенные с одним подчеркиванием, приватные - с двумя. Спереди или сзада - тут уже вкусовщина.

Но вот беда. У нас проблемы, Хьюстон.

В С++ есть определенные навязанные ограничения на нейминг сущностей со стороны underscore. Стандарт говорит:

Certain sets of names and function signatures 
are always reserved to the implementation:

- Each name that contains a double underscore (__) or
begins with an underscore followed by an uppercase letter 
is reserved to the implementation for any use.
- Each name that begins with an underscore is reserved 
to the implementation for use as a name in the global namespace.

Such names are also reserved in namespace ::std (17.4.3.1).

Имплементация загробастала себе права на имена сущностей, которые содержат двойное подчеркивание или начинаются с одного подчеркивания и буквы верхнего регистра, в любых скоупах. А также для глобального неймспейса зарезервированы имена начинающиеся на одинарный underscore.

Итого, чтобы не было коллизии имен, лучше избегать использования имен с префиксом подчеркивания или содержащих двойной underscore.

Закрытые поля можно помечать одной черточкой с обеих сторон. Только если решите использовать спереди - не пишите дальше заглавную букву. И будет вам счастье.

В питоне кстати именно через нижнее подчеркивание помечаются "непубличные" части API. Но там правда есть один нюанс. Зацените веселый мем по этой теме. Так что сегодня мем про питухон.

Be happy. Stay cool.

#cppcore

​​CamelCase vs Under_Score

Вдохновился прошлым постом и родилось это.

Есть такое ощущение, что программирование - одна из самых холиварных специальностей в мире. А в добавок к этому программисты по своему складу характера зачастую сами по себе ярые холиварщики. Это взрывоопасное комбо, которое приводит к смешным для стороннего человека проблемам.

Одна из них - как записывать многословные переменные. Уже десятилетиями люди спорят и никак не могут выработать один универсальный вариант. Поэтому приходится писать этот пост, чтобы во всем разобраться.

В настоящее время существует много стандартов наименования переменных, но два из них являются наиболее популярными среди программистов: это camel case («Верблюжья» нотация) и underscore (именование переменных с использованием символа нижнего подчеркивания в качестве разделителя).

Верблюжья нотация является стандартом в языке Java и в его неродственнике JavaScript, хотя ее можно встретить также и в других местах. Согласно этому стандарту, все слова в названии начинаются с прописной буквы, кроме первого. При этом, естественно, не используется никаких разделителей вроде нижнего подчеркивания. Пример: яШоколадныйЗаяцЯЛасковыйМерзавец. Обычно данный стандарт применяют к именам функций и переменных, при этом в именах классов, структур, интерфейсов используется стандарт UpperCamelCase(первая буква заглавная).

В стандарте underscore слова пишутся с маленькой буквы, а между ними стоит _ ( типа такого: йоу_собаки_я_наруто_узумаки). Обычно этот стандарт используется в названиях функций и переменных, а для названий классов, структур, интерфейсов используется стандарт UpperCamelCase. Обычно используется с родственных С языках.

Каждый из этих двух стандартов имеет свои сильные и слабые стороны. Вот основные:

👉🏿 Нижнее подчеркивание лучше читается: сравните стандарт_с_нижним_подчеркиванием и стандартНаписанияПрописнымиБуквами

👉🏿 Зато camel case делает более легким чтение строк, например:
my_first_var=my_second_var-my_third_var
и
myFirstVar=mySecondVar-myThirdVar

Очевидно, что camel case читается лучше: в случае с нижним подчеркиванием и оператором «минус» выражение с первого взгляда вообще можно принять за одну переменную.

👉🏿 Подчеркивание сложнее набирать. Даже при наличии intellisense, во многих случаях необходимо набирать символ нижнего подчеркивания. И имена получаются длиннее.

👉🏿 Camel Case непоследователен, потому что при использовании констант (которые иногда пишутся целиком заглавными буквами) нам приходится использовать нижнее подчеркивание. С другой стороны, стандарт underscore может быть полным, если вы решите использовать в названиях классов (структур, интерфейсов) нижнее подчеркивание в качестве разделителя.

👉🏿 Кроме того, для камел кейса не так уж и просто работать с аббревиатурами, которые обычно представлены в виде заглавных букв. Например, как вот правильно iLoveBDSM или iLoveBdsm?. Непонятно. Можете написать в комментах, как это по-вашему пишется)

В плюсах реально во многих проектах(и по моему опыту в частности) склоняются к использованию подчеркиваний в именах переменных и функций, а UpperCamelCase используется для имен классов. Но почему-то это нигде особо не упоминается и этому нигде не учат. Это как-то само приходит со временем.Хотя на начальном этапе карьеры, когда проходят первые ревью, то много непоняток в голове по поводу именований. Поэтому, вообще говоря, лучше заранее с командой обсудить этот вопрос и выработать ваш подробный код стайл, чтобы все были на одной волне по этому вопросу.

Расскажите в комментах, какую нотацию вы используете? Интересно иметь большую репрезентативную статистику.

Choose your style. Stay cool.

#fun

​​Перегружаем оператор взятия адреса
#опытным

Перегрузка операторов - дело не для слабонервных. Этим пугают детей и пытают людей. Только прожженные плюсовики могут сходу сказать разницу в перегрузке префиксного и постфиксного декремента/инкремента.

Но не только в этом дело. Есть такие операторы, которые как бы можно перегружать, но непонятно, для чего это делать.

Вот например, оператор взятия адреса. Да его можно перегружать. Можно его перегружать как метод:

struct Class {
  int* operator &() {
    return &member;
  }
  
  int member;
};

Можно, как свободную функцию:

struct A {
  int member;
};

int* operator &(Class& obj) {
  return &obj.member;
}

Но странновато это все.

Есть объект. У него есть занимаемое место. Зачем вообще заменять такое интуитивно понятное поведение?

Решение сделать этот оператор кастомным очень спорное. В реальном коде вы такого скорее всего никогда не встретите. А если подумаете, что неплохо было бы в своем коде заюзать эту фичу, то подумайте еще раз 300.

Но все-таки если оно есть, значит кто-то этим пользуется.

Например, для врапперов. Вы написали обертку для класса и хотите прозрачно передать контроль над объектами класса в обертку. Тогда вам возможно может понадобиться возвращать из оператора & указатель на внутренний объект, а не на объект враппера.

Однако тогда встает вопрос, как нормально взять адрес у обертки, если это пригодиться?

Но и на этот вопрос есть ответ. Но обсудим мы его несколько позже.

Однако уже сейчас видно, что для пользователя кода создаются серьезные препятствия для его понимания и использования. Если нужно из обертки доставать адрес оборачиваемого объекта - сделайте геттер, как у умных указателей и не парьтесь с перегрузкой.

Follow the beaten path. Stay cool.

#cppcore

Addressof
#опытным

Говорят вот, что питон - такой легкий для входа в него язык. Его код можно читать, как английские английский текст. А вот С/С++ хаят за его несколько отталкивающую внешность. Чего только указатели стоят...

Кстати о них. Все мы знаем, как получить адрес объекта:

int number = 42;
int * p_num = &number;

Человек, ни разу не видевший код на плюсах, увидит здесь какие-то магические символы. Вроде число, а вроде какие-то руны * и &. Но плюсы тоже могут в читаемость! Причем именно в аспекте адресов.

Вместо непонятного новичкам амперсанда есть функция std::addressof! Она шаблонная, позволяет получить реальный адрес непосредственно самого объекта и доступна с С++11. Для нее кстати удалена перегрузка с const T&&

template< class T >
T* addressof( T& arg ) noexcept;

template< class T >
const T* addressof( const T&& ) = delete;

Это делает функцию еще одним примером использования константной правой ссылки .

Это конечно круто, что можно в плюсах словами брать адрес, но в чем прикол? Зачем было заводить отдельную функцию для того, что уже есть в самом языке?

А вот теперь мы возвращаемся к предыдущему посту про перегрузку оператора взятия адреса. Так как его можно перегружать, то мы можем возвращать вообще любой адрес, который потенциально никак не связан с самим объектом. В этом случае не очень понятно, как взять трушный адрес объекта. Как раз таки std::addressof - способ получить валидный адрес непосредственно самого объекта.

Также большим преимуществом является шаблонная природа функции. Это позволяет обобщенному коду работать, как с обычными классами, так и с классами, у которых перегружен оператор взятия адреса.

А с С++17 она еще и констэкспр, это для любителей компайл-тайма.

Вот вам примерчик:

template<class T>
struct Ptr
{
    T* pad; // add pad to show difference between 'this' and 'data'
    T* data;
    Ptr(T* arg) : pad(nullptr), data(arg)
    {
        std::cout << "Ctor this = " << this << '\n';
    }
 
    ~Ptr() { delete data; }
    T** operator&() { return &data; }
};
 
template<class T>
void f(Ptr<T>* p)
{
    std::cout << "Ptr   overload called with p = " << p << '\n';
}
 
void f(int** p)
{
    std::cout << "int** overload called with p = " << p << '\n';
}
 
int main()
{
    Ptr<int> p(new int(42));
    f(&p);                // calls int** overload
    f(std::addressof(p)); // calls Ptr<int>* overload, (= this)
}

// OUTPUT
// Ctor this = 0x7fff59ae6e88
// int** overload called with p = 0x7fff59ae6e90
// Ptr   overload called with p = 0x7fff59ae6e88

Здесь какие-то злые персоналии перегрузили оператор взятия адреса у класса Ptr так, чтобы он возвращал указатель на одно из его полей. Ну и потом сравнивают результат работы оператора с результатом выполнения функции std::addressof.

Видно, что трушный адрес объекта, полученный с помощью this и адрес, возвращенный из std::addressof полностью совпадают. А перегруженный оператор возвращает другое значение.

Express your thoughts clearly. Stay cool.

#cpp #cpp11 #cpp17

Проверяем вхождение элемента в ассоциативный контейнер

Нужно вот нам по ключу проверить вхождение элемента допустим в мапу.

Обычно мы пишем:

if (map.count(key)) {
  // do something
}

Но для контейнеров без приставки "multi" это выглядит довольно странно. Действительно, если я знаю, что в мапе однозначное соответствие ключа и значения, зачем мне знать сколько вхождений элементов с этим ключом? Я хочу просто знать, есть ли он.

Такие вот маленькие семантические несостыковочки. С ними вроде все смирились, но осадочек остался...

И 20-е плюсы наконец нам подарили замечательный публичный метод для всех ассоциативных контейнеров contains. Он проверяет, если ли в контейнере элементы с данным ключом. Теперь код выглядит так:

if (map.contains(key)) {
  // do something
}

И вот уже стало чуть приятнее и понятнее читать код.

Если есть доступ к 20-м плюсам, то переходите на использование этого метода.

Make things clearer. Stay cool.

#STL #cpp20