Pull to refresh

Comments 36

Достаточно было примера использования и пары картинок + описание ньюансов использования. Ссылка на github очень полезна
>>В рамках одного класса реализуется и работа с серверными каналами (акцепт) и работа с клиентскими.
А почему не было разделено на два класса Читатель\Писатель?
Во время проектирования мы решили, что не будем создавать отдельный класс только ради акцепта новых клиентов. Хотя такое разделение было бы неплохо сделать. В .NET оно есть. Но там в серверный пайп можно и писать и читать.
Ваш сервер практически не содержит общего кода с клиентом да и используется только внутри библиотеки. Явно неплохо бы было разбить на 2. Плюс вместо наследованния лучше-бы было использовать агрегацию. Кажется что код от этого бы стал легче.
Насчет агрегации — не уверен, но про разбивку подумаем, действительно, смешивать функционал в одном классе нехорошо.
хотите я вам пул реквест с агрегацией запилю? :)
Было бы здорово, именно для этого я и сделал репозиторий на гитхабе, а не просто выложил zip с сорцами
А почему не использовали boost.asio, там это уже все есть:
Для unix
Для win32: 1 или 2

Также, если у вас текстовый протокол, то у вас из коробки бы работали asio streams, вместо чтения записи побайтно
Мы хотели максимально избавить внешние компоненты от сторонних зависимостей. Но, честно говоря, наличие указанных вами классов в asio мы проглядели. Спасибо за наводку.
По замечаниям: вы пишете что у вам требовалась высокая производительность, хотя по факту в коде сервера встречаются строки вида: boost::this_thread::sleep(boost::posix_time::milliseconds(100)); Что убивает ваше стремление к производительности полностью.

Для того чтобы избежать ненужного поллинга можно заменить тип очереди на concurrent_bounded_queue Тогда вы избавить от постоянной долбежки функции try_pop и будете дожидаться сообщения без поллинга.

Еще очень смущает наличие кода вида:
catch(const std::exception& e)
{
}
catch(...)
{
}

Теряется всякая информация об ошибках, что не есть хорошо.

Также есть несколько логических ошибок в коде (NamedPipeServer.cpp:12) — почему если будет выброшено исключение, то вы все равно продолжаете работу и пытаетесь обрабатывать сообщения. По моему гораздо логичней передавать в объект класса NamedPipeServer уже созданное соединение (или создавать в конструкторе), тогда бы вы не нарушали внутренних инвариантов класса (ваш объект _pipe был бы всегда создан и активен).

Ну вот вроде все.

P.S. Рекомендую прочесть книгу Энтони Вильямся concurrency in action c++. Там все эти моменты тщательно разобраны.
Спасибо за замечания и книгу.

>если будет выброшено исключение, то вы все равно продолжаете работу и пытаетесь обрабатывать сообщения.

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

>Теряется всякая информация об ошибках, что не есть хорошо.

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

>в коде сервера встречаются строки вида: boost::this_thread::sleep(boost::posix_time::milliseconds(100)); Что убивает ваше стремление к производительности полностью.

Хорошо, сделаем засыпание на 1мс. Но оно все равно должно быть — иначе поедание ресуров процессора на холостом цикле будет слишком большим
Так, давайте еще раз.
Не делайте засыпание на 1 мс. Это еще хуже, да — вы уменьшите время отклика, но если ваша очередь будет много времени пустой, то все равно будет тратиться время на вызовы try_pop. В лучшем случае вы не просадите производительность. Рекомендую сделать упор именно на concurrent_bounded_queue

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

Теперь рассмотрим другой вариант, конструктор NamedPipeServer принимает один аргумент — pipe (который должен быть создан выше по коду). Таким образом в момент создания объекта NamedPipeServer, вы уже точно знаете, что пайп открыт и метод run можно спокойно вызывать, и более того, вся эта чехарда с исключениями из метода run просто выкинется (как и вызов ConnectOrOpen. И теперь нам уже не страшно исключение, потому что мы можем написать код такого вида:
try {
  std::auto_ptr<NamedPipe> pipe(new NamedPipe(name, 1));
  NamedPipeServer server(pipe);
  server.Start();
} catch(const std::exception & e) {
  //logging, shutting down, anything else
}


Кстати, что касается исключений, то опять же, можно запускать не сами boost::thread, а использовать boost::future + boost::packaged_task, таким образом вы всегда сможете переместить исключение из потока в основной (main) поток программы.
Да, спасибо, как я написал ниже про засыпание — я просто невнимательно прочитал ваш комментарий.

Отдельное спасибо за boost::future и packaged_task — мы давно хотели найти более удобный планировщик задач.
Мне кажется, что в случае использования данной очереди возможна следующая проблема.

В документации написано, что метод pop() «Block until an item becomes available, and then dequeue it.».

Давайте представим, что все потоки-обработчики клиентов встали на методе pop() и ждут, когда в очереди что-то появится. А в это время мы хотим остановить сервер. Поток с WaitForConnection() мы уже остановили, новых клиентов нет, а другие потоки по-прежнему стоят на блокирующем pop() и, следовательно, вызов join() применительно к ним просто повесит главный поток выполнения.
UPD: Нашел метод abort(), с помощью которого можно остановить все потоки корректно.
>Для того чтобы избежать ненужного поллинга можно заменить тип очереди на concurrent_bounded_queue Тогда вы избавить от постоянной долбежки функции try_pop и будете дожидаться сообщения без поллинга.

Извиняюсь, проглядел данный совет. В принципе я изначально был за, но полгода назад, во время рождения данной библиотеки, начальство было против данной замены (
> начальство было против данной замены (
Не повезло, сочувствую.

Кстати я как-то на работе нечаянно провел тест: я написал одну программу — демон, и забыл ее выключить (там тоже были sleep и все такое) и она крутилась на тестовом сервере и все про нее забыли (случайно наткнулся на нее), я посмотрел, что за несколько месяцев работы, она сожрала день процессорного времени — а это был только простой, по факту она ничего не делала. При этом висящий рядом демон ftpd сожрал не более нескольких минут процессорного времени. Меня эта ситуация в свое время очень мотивировала на обучение параллельному программированию, возможно это вам поможет
>Не повезло, сочувствую.

Ну так все прогеры уже разбежались, и остался лишь «сам себе IT директор»
Поскольку имя канала в нашем классе хранится в std::string, то для корректной его передачи в WinAPI функции надо устанавливать в свойствах проекта в Visual Studio «Use Multi-Byte character Set»

Шел суровый 2013 год… программисты бились с кодировками как могли… а могли они немного…

//и дело даже не в том что std::string-у никто UCS-2 хранить не запрещает (ибо оно использует доп поле для определения свой длины)… но, ребята, это снежный зверек…
ну точно не так. У вас еще нет проблем с русскими именами компьютеров (к примеру) при использовании вашей «либы» с компьютера, где нет русской локали (не настроена как «по умолчанию для программ не поддерживающих unicode» (как-то так называется опция))? Тогда mr. ЗлойБаг идёт к вам…
С именами машин проблемы и не должно быть — по ТЗ и система и наше приложение должны быть развернуты на одной машине. Иначе бы мы использовали только сокеты и этой статьи не было бы.

Кроме этого, есть соглашение что имена пайпов должны быть на английском и хардкодятся в бинарниках. Ну, есть, конечно, опция с добавлением префиксов (если одна копия приложения на машине слушает, скажем 1ску а вторая — оракл), но там мы явно применяем конвертацию юникода в ascII, а в случае с 1с — еще и транслит.
Задаю этот вопрос на хабре уже второй раз — первый был оставлен сообществом без ответа. Почему так популярна среди разработчиков логическая цепочка, подобная этой:
кроме того нам не хотелось раздувать проект использованием тяжелых сторонних библиотек (на тот момент мы уже использовали boost и Intel TBB, притягивать же еще Qt или gSoap нам не хотелось).
Поэтому было решено писать свой велосипед.

Они, простите, кусаются? Библиотеки, я имею в виду. Существует диктуемое Великим Эфиром (Большим Братом / <любым другим божеством по вкусу>) ограничение на число используемых в проекте библиотек? Их затем и создают, библиотеки, чтобы можно было для решения какой-либо задачи ограничиться затратами на изучение этой самой библиотеки, а не вбухать всегда, чтобы там ни думали сами разработчики, всегда большее время на продумывание, разработку, отладку и поддержку своего решения. «тяжелых… библиотек» — вам их носить куда-то? Или пара мегабайт дискового пространства пользователя стоит дороже недель/месяцев/лет труда команды профессиональных программистов? Непонятна мне эта философия. Поясните, пожалуйста, в чем я до сих пор заблуждаюсь.
К частной сути самого поста этот комментарий большого отношения не имеет, он относится именно к общей идее, почему-то крайне распространенной в ИТ. Заранее спасибо.
Не берусь судить за автора, но имею пару своих мыслей на этот счет. Использовать большую чужую библиотеку с одной стороны хорошо и привлекательно, особенно если она решает твою проблему. С другой стороны еще привлекательней попробовать решить проблему самому, дабы научиться чему-то новому и столкнуться с новыми трудностями, и потом уже, с высоты приобретенного опыта смотреть на что-то реализованное в boost-e или другой библиотеке. Сравнивать подходы, смотреть на красивые или не очень решения. Без опыта решения задач, которые решает библиотека тяжело впитать понимание как она должна использоваться, а это уже сильно повышает риск прострелить себе ногу в нескольких местах.
Очень хороший ответ, спасибо.
Исключительно с целью стимуляции развития мысли:

Возьмем в качестве примера функциональность, реализуемую библиотекой Boost.Asio. Пусть разработчик не имеет ни большого опыта в области сетевого программировния, ни исчерпывающих знаний предмета (но базовые представления он имеет). У него есть, на мой взгляд, три пути решения сетевой задачи:
1. Просто начать использовать Boost.Asio, опираясь на ее прекрасную документацию.
2. Изучить У. Стивенса («UNIX. Разработка сетевых приложений»), изучить второй том POSA, попытаться самостоятельно реализовать библиотеку для решения своих задач, попытаться с ее помощью, собственно, решить задачу, обжечься, итеративно повторить процесс несколько раз. Затем выполнить пункт 1.
3. Прочесть несколько первых глав Стивенса. Изучить документацию Boost.Asio, при каждом подходящем случае (и даже чаще) обращаясь за детальными разъяснениями к Стивенсу и POSA. Обеспечить себе таким образом a) глубокое понимание архитектуры используемой библиотеки b) хорошее представление об ограничениях библиотеки и местах, в которых её абстракции могут дать течь. Выполнить пункт 1.

Мне представляется, что путь 3) наиболее разумен в большинстве случаев.

И да. Вы попытались объяснить, зачем изобретают велосипеды. Но пока остается совсем непонятным, почему это желание сами изобретатели объясняют не так, как вы, а так, как автор. Ссылаясь на противоестественность использования более, чем n библиотек в проекте, «тяжесть» чего-либо, измеряемую непонятно в чём, и пр.
Для ответа на этот вопрос мне проще всего восстановить по памяти диалог полугодовой давности:

— как мы будем связываться из внешней компоненты с нашим проектом?

— сокеты

— не айс, loopback это — медленно, посмотри что-нибудь вроде shared memory

— вот есть межпроцессная очередь сообщений в бусте

— она с приоритетами, это тоже медленно.

— ну вот когда я делал компоненту для MSSQL, то там я использовал именованные каналы из .NET

— поищи такое же для плюсов

— нашел только QLocalsocket в Qt

(пауза)

— это слишком толстая либа, ставить ее на все компы заказчика не айс. Может, попробуем свою написать?

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

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

Только при написании этой статьи я убрал 6 багов из той версии, что сейчас крутится у заказчика. Ошибки были не критичные — при определенных условиях сервер посылал клиент после соединения, но клиент умел переподключаться, да пара утечек памяти (одна в деструкторе сервера, который никогда не вызывался).
Большие библиотеки как минимум увеличиваю время сборки проекта. Да и баги (если найдутся) в них тяжело фиксить.
В этом отношении можно понять любителей маленьких велосипедов.
К вороху советов выше хочу добавить, что использовать xml для коммуникации и заботиться о производительности одновременно — это как-то не правильно. :) Мы для обмена данными начали использовать MessagePack вместо xml и остались очень довольны: он компактный, быстро парсится, легко читается и легко пишется. Есть код для работы с MessagePack из основных языков.
Заказчику очень хочется посылать именно xml. Причем, по замыслу проекта, он просто гоняет эти xml между разными системами, никак их не меняя и даже не читая.
Messagepack мы используем для мериализации класса сообщений, которые передаем по сети.
Он в курсе. MessagePack, кстати, вполне может заменить BSON.
Проблема тут в том, что тот же 1с или MSSQL средствами встроенного языка умеет формировать и парсить только xml-ки. Поэтому в наше приложение из обозначенных систем приходит именно xml. Так как наше приложение фактически без изменений засылает его в другую (но удаленную) систему, то особого резона преобразовывать xml<->BSON мы не видели. Разве что трафик экономить.
Простите мозги уже в кучу, но как вы прерываете работу клинта если он висит на операции read если это Linux система?
В принципе, в boost есть возможность получить нативный указатель на поток. После этого можно сделать pthread_kill() или CancelSynchronousIo() в Windows Vista и выше, но у заказчика еще местами XP стоит :(.
Еще один вариант — крутить в цикле select() перед вызовом recv().
Но по-хорошему надо работать с неблокирующими (асинхронными) операциями.
Есть еще один вариант, без убийства потоков.Надо сделать возможность получения указателя на канал, который обрабатывается потоком сервера. Тогда достаточно закрыть данный канал для того чтобы прервать операции чтения. Нехорошего тут только одно — пострадают те клиенты, с которыми происходит обмен данными.
Почему вы не рассматривали ZeroMQ например?
Честно говоря, тогда (полгода назад) никто из программистов, включая начальника, не предложил такую идею. Только стандартные варианты — sockets, shared memory, pipes.
Но сейчас работы по переходу на MQ уже ведутся
Sign up to leave a comment.

Articles

Change theme settings