В моей новой команде, которой я управляю, большая часть нашего кода - это платформа, сокет TCP и код сети http. Все на С ++. Большая часть этого произошла от других разработчиков, которые покинули команду. Нынешние разработчики в команде очень умные, но в основном младшие с точки зрения опыта.
Наша самая большая проблема: многопоточные ошибки параллелизма. Большинство наших библиотек классов написаны как асинхронные с использованием некоторых классов пула потоков. Методы в библиотеках классов часто ставят в очередь долго выполняющиеся такты в пул потоков из одного потока, а затем методы обратного вызова этого класса вызываются в другом потоке. В результате у нас есть много ошибок в крайнем случае, связанных с неверными предположениями о потоке. Это приводит к тонким ошибкам, которые выходят за рамки просто наличия критических секций и блокировок для защиты от проблем параллелизма.
Что делает эти проблемы еще сложнее, так это то, что попытки исправить их часто неверны. Некоторые ошибки, которые я наблюдал при попытке команды (или внутри самого унаследованного кода), включают в себя что-то вроде следующего:
Распространенная ошибка № 1 - Устранение проблемы параллелизма, просто установив блокировку общих данных, но забывая о том, что происходит, когда методы не вызываются в ожидаемом порядке. Вот очень простой пример:
void Foo::OnHttpRequestComplete(statuscode status)
{
m_pBar->DoSomethingImportant(status);
}
void Foo::Shutdown()
{
m_pBar->Cleanup();
delete m_pBar;
m_pBar=nullptr;
}Так что теперь у нас есть ошибка, из-за которой Shutdown может вызываться, пока происходит OnHttpNetworkRequestComplete. Тестировщик находит ошибку, фиксирует аварийный дамп и назначает ошибку разработчику. Он в свою очередь исправляет ошибку, как это.
void Foo::OnHttpRequestComplete(statuscode status)
{
AutoLock lock(m_cs);
m_pBar->DoSomethingImportant(status);
}
void Foo::Shutdown()
{
AutoLock lock(m_cs);
m_pBar->Cleanup();
delete m_pBar;
m_pBar=nullptr;
}Вышеуказанное исправление выглядит хорошо, пока вы не поймете, что есть еще более тонкий случай. Что произойдет, если Shutdown вызывается до того, как OnHttpRequestComplete будет вызван обратно? Примеры из реальной жизни, которые есть у моей команды, еще сложнее, а крайние случаи еще сложнее обнаружить в процессе проверки кода.
Распространенная ошибка № 2 - устранение проблем взаимоблокировки путем слепого выхода из блокировки, ожидания завершения другого потока, а затем повторного входа в блокировку - но без обработки случая, когда объект только что обновился другим потоком!
Распространенная ошибка № 3 - Несмотря на то, что объекты подсчитаны, последовательность выключения «освобождает» свой указатель. Но забывает ждать, пока поток, который все еще работает, освобождает свой экземпляр. Таким образом, компоненты полностью отключаются, а затем ложные или поздние обратные вызовы вызываются для объекта в состоянии, не ожидающем больше вызовов.
Есть и другие крайние случаи, но суть заключается в следующем:
Многопоточное программирование просто сложно, даже для умных людей.
Когда я улавливаю эти ошибки, я трачу время на обсуждение ошибок с каждым разработчиком для разработки более подходящего исправления. Но я подозреваю, что они часто путаются в том, как решить каждую проблему из-за огромного количества унаследованного кода, который «правильное» исправление будет включать в себя касание.
Мы скоро будем в продаже, и я уверен, что патчи, которые мы применяем, сохранятся для предстоящего релиза. После этого у нас будет время улучшить базу кода и рефакторинг, где это необходимо. У нас не будет времени просто переписать все. И большая часть кода не так уж и плоха. Но я пытаюсь реорганизовать код так, чтобы можно было избежать проблем с многопоточностью.
Один подход, который я рассматриваю, заключается в следующем. Для каждой важной функции платформы выделите отдельный поток, в который будут перенаправляться все события и сетевые обратные вызовы. Похоже на многопоточность COM-квартир в Windows с использованием цикла сообщений. Длинные операции блокировки могут все еще передаваться потоку рабочего пула, но обратный вызов завершения вызывается в потоке компонента. Компоненты могут даже иметь общий поток. Тогда все библиотеки классов, работающие внутри потока, могут быть написаны в предположении о едином поточном мире.
Прежде чем идти по этому пути, мне также очень интересно, есть ли другие стандартные методы или шаблоны проектирования для решения многопоточных проблем. И я должен подчеркнуть - нечто большее, чем книга, которая описывает основы мьютексов и семафоров. Что вы думаете?
Я также заинтересован в любых других подходах к процессу рефакторинга. Включая любое из следующего:
Литература или статьи о дизайне узоров вокруг ниток. Что-то за пределами введения в мьютексы и семафоры. Нам не нужен массивный параллелизм либо, только пути для разработки объектной модели таким образом , чтобы обрабатывать асинхронные события из других потоков правильно .
Способы составления схемы потоков различных компонентов, чтобы было легко изучать и разрабатывать решения. (То есть UML-эквивалент для обсуждения потоков между объектами и классами)
Обучение вашей команды разработчиков по вопросам с многопоточным кодом.
Чтобы ты делал?
источник
Ответы:
В вашем коде есть и другие важные проблемы, кроме этого. Удалить указатель вручную? Вызов
cleanupфункции? Owch. Кроме того, как точно указано в комментарии к вопросу, вы не используете RAII для блокировки, что является еще одним довольно грандиозным отказом и гарантирует, что приDoSomethingImportantвозникновении исключения происходят ужасные вещи.Тот факт, что эта многопоточная ошибка возникает, является лишь симптомом основной проблемы - ваш код имеет очень плохую семантику в любой ситуации с потоками, и вы используете совершенно ненадежные инструменты и экс-идиомы. Если бы я был тобой, я бы удивился тому, что он работает с одним потоком, не говоря уже о большем.
Весь смысл подсчета ссылок в том, что поток уже выпустил свой экземпляр . Потому что если нет, то он не может быть уничтожен, потому что поток все еще имеет ссылку.
Использование
std::shared_ptr. Когда все потоки выпустили (и никто , следовательно, не может быть вызовом функции, так как они не имеют указателя на него), то вызывается деструктор. Это гарантированно безопасно.Во-вторых, используйте настоящую библиотеку потоков, такую как Intel Thread Building Blocks или Microsoft Parallel Patterns Library. Написание собственного занимает много времени и ненадежно, а ваш код полон многопоточности деталей, которые ему не нужны. Выполнение ваших собственных блокировок так же плохо, как и собственное управление памятью. Они уже реализовали много очень полезных идиом потоков общего назначения, которые работают правильно для вашего использования.
источник
Другие авторы хорошо прокомментировали, что нужно сделать, чтобы исправить основные проблемы. Этот пост посвящен более актуальной проблеме исправления устаревшего кода, достаточной для того, чтобы выиграть время, чтобы все переделать правильным образом. Другими словами, это не верный способ делать вещи, это просто способ хромать пока.
Ваша идея консолидации ключевых событий - хорошее начало. Я бы пошел так далеко, чтобы использовать один поток диспетчеризации для обработки всех ключевых событий синхронизации, где бы ни была зависимость от порядка. Настройте потокобезопасную очередь сообщений и везде, где вы в данный момент выполняете чувствительные к параллелизму операции (выделения, очистки, обратные вызовы и т. Д.), Вместо этого отправьте сообщение в этот поток и попросите его выполнить или инициировать операцию. Идея состоит в том, что этот поток управляет всеми запусками, остановками, выделениями и очистками рабочего блока.
Поток рассылки не решает описанные вами проблемы, он просто объединяет их в одном месте. Вам все еще нужно беспокоиться о событиях / сообщениях, происходящих в неожиданном порядке. События со значительным временем выполнения по-прежнему необходимо отправлять в другие потоки, поэтому по-прежнему существуют проблемы с параллелизмом для общих данных. Один из способов смягчения - избежать передачи данных по ссылке. По возможности данные в сообщениях об отправке должны быть копиями, которые будут принадлежать получателю. (Это делается для того, чтобы сделать данные неизменяемыми, о которых упоминали другие.)
Преимущество этого подхода к диспетчеризации заключается в том, что в потоке диспетчеризации у вас есть своего рода безопасное убежище, где вы, по крайней мере, знаете, что определенные операции выполняются последовательно. Недостатком является то, что это создает узкое место и увеличивает нагрузку на процессор. Я предлагаю сначала не беспокоиться ни об одной из этих вещей: сконцентрируйтесь на том, чтобы сначала получить некоторую меру правильной работы, перемещаясь как можно больше в поток рассылки. Затем выполните профилирование, чтобы увидеть, что занимает больше всего процессорного времени, и начните перемещать его обратно из потока диспетчеризации, используя правильные методы многопоточности.
Опять же, то, что я описываю, это не правильный способ сделать что-то, а процесс, который может привести вас к правильному пути с шагом, который достаточно мал, чтобы соответствовать коммерческим срокам.
источник
Основываясь на показанном коде, у вас есть куча WTF. Крайне сложно, если не невозможно, постепенно исправить плохо написанное многопоточное приложение. Скажите владельцам, что приложение никогда не будет надежным без значительных переделок. Дайте им оценку, основанную на проверке и переработке каждого бита кода, который взаимодействует с общими объектами. Сначала дайте им оценку для осмотра. Затем вы можете дать оценку для переделки.
Когда вы переделываете код, вы должны планировать писать код так, чтобы он был корректно корректным. Если вы не знаете, как это сделать, найдите того, кто это сделает, или вы окажетесь в том же месте.
источник
Если у вас есть время посвятить рефакторингу вашего приложения, я бы посоветовал вам взглянуть на модель актера (см., Например, Theron , Casablanca , libcppa , CAF для реализаций C ++).
Актеры - это объекты, которые работают одновременно и взаимодействуют друг с другом только с помощью асинхронного обмена сообщениями. Итак, все проблемы управления потоками, мьютексов, взаимоблокировок и т. Д. Решаются библиотекой реализации акторов, и вы можете сосредоточиться на реализации поведения ваших объектов (актеров), которое сводится к повторению цикла
Один из подходов для вас - сначала прочитать эту тему и, возможно, взглянуть на одну или две библиотеки, чтобы узнать, можно ли интегрировать модель актора в ваш код.
Я использую (упрощенную версию) эту модель в своем проекте уже несколько месяцев, и я поражен тем, насколько она надежна.
источник
Ошибка здесь не в «забывании», а в «не исправлении». Если у вас что-то происходит в неожиданном порядке, у вас есть проблема. Вы должны решить это вместо того, чтобы пытаться обойти это (наложение блокировки на что-то обычно обходное решение).
Вы должны попытаться адаптировать модель актера / обмен сообщениями в определенной степени и разделить заботу. Роль
Fooсостоит в том, чтобы обрабатывать какие-то HTTP-коммуникации. Если вы хотите спроектировать свою систему так, чтобы она выполняла это параллельно, то вышеприведенный уровень должен обрабатывать жизненные циклы объектов и соответственно обращаться к синхронизации.Попытка заставить несколько потоков работать с одними и теми же изменяемыми данными трудна. Но это также редко необходимо. Все общие случаи, которые требуют этого, уже были абстрагированы в более управляемые концепции и реализованы несколько раз для любого основного императивного языка. Вы просто должны их использовать.
источник
Ваши проблемы довольно плохие, но типичные для плохого использования C ++. Обзор кода исправит некоторые из этих проблем. 30 минут, один набор глазных яблок дает 90% результатов. (Цитата для этого является googleable)
# 1 Проблема Вы должны убедиться, что существует строгая иерархия блокировки, чтобы предотвратить блокировку блокировки.
Если вы замените Autolock на обертку и макрос, вы можете сделать это.
Сохраняйте статическую глобальную карту замков, созданную в конце вашей оболочки. Макрос используется для вставки информации о названии и номере строки в конструктор оболочки Autolock.
Вам также понадобится статический доминантный граф.
Теперь внутри блокировки вы должны обновить график доминатора, и если вы получите изменение порядка, вы выдадите ошибку и прервитесь.
После тщательного тестирования вы можете избавиться от большинства скрытых тупиков.
Код оставлен в качестве упражнения для студента.
Проблема № 2 затем исчезнет (в основном)
Ваше архитектурное решение будет работать. Я использовал это раньше в миссионерских и жизненно важных системах. Мой взгляд на это
Не делитесь данными через публичные переменные или получатели.
Внешние события поступают через многопоточную диспетчеризацию в очередь, обслуживаемую одним потоком. Теперь вы можете рассуждать об обработке событий.
Изменения данных, которые пересекают потоки, попадают в потокобезопасную очередь и обрабатываются одним потоком. Сделать подписку. Теперь вы можете рассуждать о потоках данных.
Если ваши данные должны пройти через весь город, опубликуйте их в очереди данных. Это скопирует его и передаст его подписчикам асинхронно. Также нарушает все зависимости данных в программе.
Это в значительной степени актерская модель по дешевке. Ссылки Джорджио помогут.
Наконец, ваша проблема с выключенными объектами.
Когда вы подсчитываете ссылки, вы решили 50%. Остальные 50% - это обратные вызовы. Пройдите обратный звонок владельцам референций. Вызов при отключении должен ждать нуля на счету. Не решает сложные графы объектов; это входит в настоящую сборку мусора. (Что побуждает Java не давать никаких обещаний о том, когда или если будет вызван метод finalize (); чтобы вы могли таким образом выйти из программирования.)
источник
Для будущих исследователей: в дополнение к ответу об актерской модели я хотел бы добавить CSP ( связывание последовательных процессов ) с наклоном к большему семейству исчислений процессов, в котором он находится. CSP похож на модель акторов, но разделен по-другому. У вас все еще есть несколько потоков, но они взаимодействуют по определенным каналам, а не конкретно друг с другом, и оба процесса должны быть готовы отправлять и получать соответственно, прежде чем произойдет что-либо. Существует также формализованный язык для проверки правильности кода CSP. Я все еще перехожу на использование CSP, но я использую его в нескольких проектах уже несколько месяцев, и это значительно упростило процесс.
Кентский университет имеет реализацию на C ++ ( https://www.cs.kent.ac.uk/projects/ofa/c++csp/ , клонированную по адресу https://github.com/themasterchef/cppcsp2 ).
источник
В настоящее время я читаю это, и оно объясняет все проблемы, которые вы можете получить, и как их избежать, в C ++ (используя новую библиотеку потоков, но я думаю, что глобальные объяснения действительны для вашего случая): http: //www.amazon. ком / C-параллелизмом-Action-практический-Многопоточность / дп / 1933988770 / исх = sr_1_1? т = UTF8 & QID = 1337934534 & стер = 8-1
Я лично использую упрощенный UML и просто предполагаю, что сообщения выполняются асинхронно. Кроме того, это верно для «модулей», но внутри модулей я не хочу знать.
Книга поможет, но я думаю, что упражнения / прототипирование и опытный наставник были бы лучше.
Я бы полностью избегал, чтобы люди, не понимающие проблемы параллелизма, работали над проектом. Но я полагаю, что вы не можете этого сделать, поэтому в вашем конкретном случае, кроме попытки убедиться, что команда становится более образованной, я понятия не имею.
источник
Вы уже в пути, признавая проблему и активно ища решение. Вот что я бы сделал:
источник
Посмотрите на ваш пример: как только Foo :: Shutdown начнет выполняться, не должно быть возможности вызвать OnHttpRequestComplete для запуска больше. Это никак не связано с какой-либо реализацией, просто не может работать.
Вы также можете утверждать, что Foo :: Shutdown не должен вызываться, пока выполняется вызов OnHttpRequestComplete (определенно верно), и, вероятно, нет, если вызов OnHttpRequestComplete все еще не выполнен.
Первое, что нужно сделать, это не блокировка и т. Д., А логика того, что разрешено или нет. Простая модель: ваш класс может иметь ноль или более незавершенных запросов, ноль или более завершений, которые еще не были вызваны, ноль или более завершений, которые выполняются, и что ваш объект хочет завершить работу или нет.
Ожидается, что Foo :: Shutdown завершит выполнение завершений, выполнит неполные запросы до точки, где они могут быть отключены, если это возможно, чтобы не допустить запуска дополнительных завершений, чтобы не допустить запуска большего количества запросов.
Что вам нужно сделать: добавить спецификации в ваши функции, точно говоря , что они будут делать. (Например, запуск HTTP-запроса может завершиться неудачно после вызова Shutdown). А затем напишите свои функции, чтобы они соответствовали спецификациям.
Блокировки лучше всего использовать только на крошечный промежуток времени, чтобы контролировать изменение общих переменных. Таким образом, у вас может быть переменная «workingShutDown», которая защищена замком.
источник
Если честно; Я бы сбежал, быстро.
Проблемы параллелизма являются противными . Что-то может работать идеально в течение нескольких месяцев, а затем (из-за определенного времени для нескольких вещей) внезапно взорвется перед лицом клиента, без возможности выяснить, что произошло, без надежды когда-либо увидеть хороший (воспроизводимый) отчет об ошибке и без возможности даже быть уверенным, что это не аппаратный сбой, который не имеет ничего общего с программным обеспечением.
Предотвращение проблем параллелизма должно начинаться на этапе проектирования, начиная с того, как именно вы собираетесь это сделать («глобальный порядок блокировки», модель актора, ...). Это не то, что вы пытаетесь исправить в безумной панике в надежде, что после грядущего релиза все не пойдет на самоуничтожение.
Обратите внимание, что я не шучу здесь. Ваши собственные слова (« Большинство из них произошли от других разработчиков, покинувших команду. Нынешние разработчики в команде очень умны, но в основном с точки зрения опыта». ) Показывают, что все опытные люди уже сделали то, что я предлагаю.
источник