пятница, 28 июля 2023 г.

[prog] Будьте бдительны, если вам потребуется работать с CivetWeb из нескольких тредов

Есть такая чисто Си-шная разработка, CivetWeb, которая позволяет встроить HTTP/WebSocket сервер в ваше приложение. Насколько я понял, там используется модель thread-per-connection или что-то вроде того. Т.е. когда CivetWeb принимает подключение, он создает рабочую нить, на которой CivetWeb и будет работать с этим подключением.

Очень простая модель. Но в случае, если вам доведется обслуживать посредством CivetWeb WebSocket-подключения, нужно проявлять осторожность.

Дело в том, что CivetWeb на контексте своей нити (давайте называть ее IO-thread) будет дергать ваши callback-и: connect_handler (хотя для WebSocket это не так актуально), ready_handler, data_handler и close_handler.

ready_handler будет вызван когда CivetWeb примет WebSocket подключение с той стороны (т.е. после завершения процедуры upgrade protocol). По сути, с этого момента и начинается ваша работа именно с WebSocket-подключением. В ready_handler передается указатель на mg_connection, с которым вы будете иметь дело.

data_handler вызывается, когда CivetWeb вычитывает входящие данные из соединения и отдает их вам на обработку (как раз посредством вызова data_handler callback на контексте IO-thread). И если у вас протокол поверх WebSocket-а вида запрос-ответ, то вы можете сразу здесь, внутри data_handler, выполнить mg_websocket_write. В этом случае у вас все хорошо.

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

Вот здесь нужно проявить осторожность. Т.к. делать вызов mg_websocket_write вы будете на контексте какой-то своей рабочей нити, а не на контексте IO-thread от CivetWeb. А осторожность нужна из-за наличия close_handler callback-а.

Этот close_handler вызывается CivetWeb-ом на контексте IO-thread когда CivetWeb обнаруживает, что соединение разорвано (по инициативе удаленной стороны или из-за ошибки ввода-вывода). И, внимание, после завершения close_handler указателем на mg_connection пользоваться нельзя, он уже протух!

Так вот, засада в том, что у вас есть собственная пишущая нить, на которой вы можете вызывать mg_websocket_write. Но тут вашу пишущую нить приостанавливают, управление получает IO-thread, на которой вызывается close_handler, после которого mg_connection становится недействительным. И после того, как mg_connection стал недействительным, просыпается ваша пишущая нить, на которой и происходит вызов mg_websocket_write. С невалидным указателем на mg_connection, ага.

Чтобы не попадать в такую ситуацию потребуется какой-то механизм защиты для имеющегося у вас на руках mg_connection. Например, вот такой (это псевдокод, без претензии на компилябильность):

// Экземпляр этой структуры будет создаваться для каждого подключения. 
struct WriterThreadData {
  Mutex lock_;
  mg_connection * conn_;
  bool closed_;
  unsigned use_count_;
};

// Основное тело пишущей нити.
void writer_thread(WriterThreadData * thread_data) {
  bool should_countinue = true;
  bool should_free_thread_data = false;
  do {
    acquire_new_data();
    // Все действия с thread_data только при захваченном мьютексе.
    lock(thread_data->lock_);
    // Записывать можно только если канал еще не закрыт.
    if(!thread_data->closed_) {
      mg_websocket_write(thread_data->conn_, ...);
    }
    else {
      // А вот если канал закрыт, то нужно завершать работу.
      should_free_thread_data = (0 == --(thread_data->use_count_));
      should_continue = false;
    }
    // Разблокируем thread_data чтобы мог поработать кто-то еще.
    unlock(thread_data->lock_);
  } while(should_continue);

  // Очистка.
  if(should_free_thread_data) {
    free(thread_data);
  }
}

// Обработчик появления нового WebSocket-подключения.
void on_ready_handler(const struct mg_connection * conn, void *) {
  WriterThreadData * thread_data = malloc(sizeof(WriterThreadData));
  init_mutex(thread_data->lock_);
  thread_data->conn_ = conn;
  thread_data->closed_ = false;
  thread_data->use_count_ = 2// IO-thread + writer thread.

  // Запускаем нить писателя.
  start_new_thread(writer_thread, thread_data);
  // Это нужно для close_handler-а.
  mg_set_user_connection_data(conn, thread_data);
}

// Обработчик закрытия существующего WebSocket-подключения.
void on_closed_handler(const struct mg_connection * conn, void *) {
  WriterThreadData * thread_data = mg_get_user_connection_data(conn);
  bool should_free_thread_data = false;
  // Прежде чем закрыть соединение нужно захватить мьютекс.
  lock(thread_data->lock_);
  thread_data->closed_ = true;
  should_free_thread_data = (0 == --(thread_data->use_count_));
  unlock(thread_data->unlock_);

  if(should_free_thread_data) {
    free(thread_data);
  }
}

Принцип здесь такой, что работать с mg_connection можно только через вспомогательный объект thread_data и только когда захвачен мьютекс этого объекта. Благодаря этому close_handler не сможет инвалидировать mg_connection пока этот mg_connection используется в пишущей нити. А пишущая нить перед записью проверяет валидность mg_connection и тем самым обнаруживает момент, когда указатель "протухает".

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

PS. Почему мне показалось это все важным? Потому, что если вы работаете с какой-то C++ной библиотекой, вроде WebSocket++, то у вас, скорее всего, в качестве mg_connection будет какой-то из умных указателей (либо std::weak_ptr, либо сразу std::shared_ptr) и, тем самым, в C++ такой проблемы с внезапной инвалидацией указателя на соединения вы не столкнетесь. Но CivetWeb -- это чистый Си, здесь такое запросто.

4 комментария:

Stanislav Mischenko комментирует...

ИМХО странная какая-то проблема. Почему бы mg_websocket_write просто не вернуть код ошибки, если соединение закрыто? Почему мы узнаём об этом только через close_handler? Но опять же "мопед не ваш", так что это не притензия, просто мысли вслух.

eao197 комментирует...

> Почему бы mg_websocket_write просто не вернуть код ошибки, если соединение закрыто?

Потому что mg_websocket_write нужно обратиться по указателю, а это уже указатель на физически удаленный объект (его память освобождена). Классический use after free.

Stanislav Mischenko комментирует...

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

eao197 комментирует...

Библиотеки пишут для того, чтобы было удобно делать так, как удобно именно что автору библиотеки. А пользователи или будут разделять взгляды автора, либо притерпятся, либо воспользуются чем-то другим.

Для CivetWeb это просто сценарий, на который она, как я понимаю, и не была рассчитана.