LINUX.ORG.RU

И снова про многопоточность

 ,


0

2

Сделал вот такой класс двунаправленного связанного списка http://pastebin.com/JRsWwaKS. По понятным причинам он не является thread-safe. Поэтому я любые обращения к нему защищаю с помощью pthread mutex. И всегда думал - «если есть потоконебезопасный код, то защищаешь его mutex, теряешь какую-то производительность, зато работаешь как обычно». А вот нифига.

pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
List list;

void* thread_func(void* arg) {
	ListItem item;
	while (true) {
		pthread_mutex_lock(&mutex);
		list.insertHead(&item);
		pthread_mutex_unlock(&mutex);
	}
}

int main() {
	pthread_t threads[2];
	pthread_create(&threads[0], nullptr, thread_func, nullptr);
	pthread_create(&threads[1], nullptr, thread_func, nullptr);
	while (true) {
		pthread_mutex_lock(&mutex);
		ListItem* item;
		do {
			item = list.removeTail();
		} while (item);
		pthread_mutex_unlock(&mutex);
	}
	return 0;
}

Это небольшой тестовый код. Два потока добавляют элементы в список (добавление одного и того же элемента допустимо, ведь в функциях добавления есть проверка, что элемента ещё нет в списке и они просто вернут false). А другой извлекает все доступные элементы. Пример абстрактный, в реальном приложении над элементами списка ещё и совершают всякие полезные действия (а ещё иногда спят и т. д.). Почему mutex не интегрирован в класс списка? Потому что опять же в реальном коде списки являются частями более сложных структур, которые уже имеют свои mutex. Это чтобы избежать стилистических замечаний. Вышеприведённый код 100% блокирует все операции со списком, хотя и делает это не в ООП стиле.

И в итоге что я получаю? А получаю я срабатывание assert(retval) в функции List::removeTail. А это может означать лишь одно - в списке числится (и не просто числится, а числится хвостом списка) элемент, у которого m_list == nullptr. Что и подтверждается, если посмотреть отладчиком после abort из-за проваленного assert. Значит либо ошибка в реализации алгоритма списка (но в однопоточных тестах вроде проблем не было), либо я что-то делаю не так с блокировками.

Я понимаю, что скорее всего у меня возникло некоторое непонимание с тем как писать многопоточные приложения на C++ и считаю, что данный пример позволит наконец-то во всём разобраться (ибо он не сложный, кода мало и я могу легко сделать минимально рабочий пример). А ещё я собираюсь читать книгу про многопоточное программирование в C++, однако хотелось бы получить первые замечания быстрее, чем я её прочитаю.

P. S.: Тестовый код компилирую без каких-либо оптимизаций.

UPD: Посыпаю голову пеплом. Ошибка была в том что функции добавления таки не проверяли проверку item->m_list. Сейчас всё работает.

★★★★★

Последнее исправление: KivApple (всего исправлений: 2)

Ответ на: комментарий от i36_zubov

item это локальная переменная каждого потока, однако она никогда не будет уничтожена, потому что:

1) постоянно используется + у компилятора отключены оптимизации

2) потоки никогда не завершатся, потому что они состоят из бесконечного цикла

Но даже если всё равно сделать item'ы глобальными, то ошибка не исчезает (проверено).

Если вы про попытку добавить в список элемент, которым там уже есть, то я повторю - все функции добавления делают item->m_list = this, а все функции удаления item->m_list = nullptr. При этом функции добавления отказываются удалять элемент, у которого item->m_list != nullptr, а функции удаления отказываются удалять элемент у которого item->m_list == nullptr. Если предположить атомарность операций из-за mutex, то двойное добавление элемента приведёт всего лишь к возврату функцией добавления false вместо true без какой-либо модификации списка.

KivApple ★★★★★
() автор топика
Последнее исправление: KivApple (всего исправлений: 1)
Ответ на: комментарий от i36_zubov
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
List list;
ListItem items[2];

void* thread_func(void* arg) {
	ListItem* item = static_cast<ListItem*>(arg);
	while (true) {
		int retval = pthread_mutex_lock(&mutex);
		assert(retval == 0);
		list.insertHead(item);
		pthread_mutex_unlock(&mutex);
	}
}

int main() {
	pthread_t threads[2];
	pthread_create(&threads[0], nullptr, thread_func, &items[0]);
	pthread_create(&threads[1], nullptr, thread_func, &items[1]);
	while (true) {
		int retval = pthread_mutex_lock(&mutex);
		assert(retval == 0);
		ListItem* item;
		do {
			item = list.removeTail();
		} while (item);
		pthread_mutex_unlock(&mutex);
	}
	return 0;
}

Вот версия кода с глобальными переменными - валится точно также (заодно добавил проверку ошибок захвата mutex - на этих assert не валится никогда, хоть это и ожидаемо).

KivApple ★★★★★
() автор топика
Последнее исправление: KivApple (всего исправлений: 1)
Ответ на: комментарий от intelfx

Что ж с козырей то заходите.

Я надеялся дождаться вайна пострадавших от C++

i36_zubov
()
Ответ на: комментарий от intelfx

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

Если вас беспокоит, что я добавляю локальную переменную потока, то чуть выше я написал пример, где item'ы являются глобальными переменными. Результат тот же.

В любом случае mutex должен защищать от проблем, потому что в однопоточном режиме добавление одного и того же элемента работает верно (второе добавление игнорируется, а функция добавления возвращает false). А раз не защищает - я что-то не понимаю в многопоточном программировании и могу допустить другие ошибки, если не пойму как исправить этот пример.

KivApple ★★★★★
() автор топика
Последнее исправление: KivApple (всего исправлений: 1)
Ответ на: комментарий от BRE

Я уже нашёл ошибку. Я думал, что проверка в функциях добавления есть, а её сам нет. Добавил if (item->m_list != nullptr) return false в начало insertBefore и insertAfter и всё заработало.

KivApple ★★★★★
() автор топика
Ответ на: комментарий от KivApple

Заверни голову списка, мутекс и операции со списком в один класс

no-dashi ★★★★★
()
Ответ на: комментарий от KivApple

Нет, меня беспокоит то, что каждое последующее добавление уже добавленного элемента портит список, т. к. уже после второго добавления единственный элемент списка будет ссылаться на себя и prev'ом, и next'ом.

intelfx ★★★★★
()
Ответ на: комментарий от KivApple

Ты ведь понимаешь, что ты только что сделал busy loop?

intelfx ★★★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.