1. C++ / Говнокод #16271

    +18

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    23. 23
    24. 24
    25. 25
    26. 26
    27. 27
    28. 28
    29. 29
    30. 30
    31. 31
    32. 32
    33. 33
    34. 34
    35. 35
    36. 36
    37. 37
    38. 38
    39. 39
    40. 40
    41. 41
    42. 42
    43. 43
    44. 44
    45. 45
    46. 46
    47. 47
    48. 48
    49. 49
    50. 50
    51. 51
    52. 52
    53. 53
    54. 54
    55. 55
    56. 56
    57. 57
    58. 58
    59. 59
    60. 60
    61. 61
    62. 62
    63. 63
    64. 64
    65. 65
    66. 66
    67. 67
    68. 68
    69. 69
    70. 70
    71. 71
    72. 72
    73. 73
    74. 74
    75. 75
    76. 76
    77. 77
    78. 78
    79. 79
    80. 80
    81. 81
    82. 82
    83. 83
    84. 84
    85. 85
    86. 86
    87. 87
    #include <iostream>
    #include <memory>
    #include <mutex>
    #include <condition_variable>
    #include <atomic>
    
    
    typedef ::std::unique_lock<::std::mutex> TLock;
    
    using namespace std;
    template<typename T>
    class TQueueElement
    {
    public:
    	T _data;
    	std::shared_ptr<TQueueElement<T> > _prev;
    	mutable ::std::mutex _lockElem;
    
    public:
    	TQueueElement(T data):_data(data),_prev(nullptr){};
    	virtual ~TQueueElement(){};
    
    
    };
    
    template<typename T>
    class TQueue
    {
    private:
    	mutable ::std::mutex _lockHead;
    	mutable ::std::mutex _lockTail;
    	::std::condition_variable _pushToQueue;
    	std::atomic<bool> _isEmpty;
    	std::shared_ptr<TQueueElement<T> > _tail;
    	std::shared_ptr<TQueueElement<T> > _head;
    
    public:
    	TQueue():_lockHead(),_isEmpty(true){};
    	virtual ~TQueue(){};
    
    ///получаем элемент из очереди
    	T pop()
    	{
    		TLock lock(_lockTail);//блокируем все получения из очереди
    		if (_tail==nullptr) 
    			{_isEmpty=true; _pushToQueue.wait(lock,[this](){return !_isEmpty;});} //если очередь пуста ожидаем пока в ней чтото появиться 
    		{
    			TLock lockTail(_tail->_lockElem); //блокируем последний элемент в очереди возможно к нему попробуют обратиться во время запихивания в очередь
    			auto data=_tail->_data;
    			_tail=_tail->_prev;
    			return data;
    		}
    		
    	}
    
    	void push(T data)
    	{
    		auto el=std::shared_ptr<TQueueElement<T> >(new TQueueElement<T>(data));//заранее создаем элемент очереди 
    		TLock lock(_lockHead);
    		if (_isEmpty)//если очередь пуста говорим что наш элемент пока первый и последний
    		{
    			_head=el;
    			_tail=el;
    			_isEmpty=false;
    			_pushToQueue.notify_all();
    		}else//если очередь не пуста 
    		{
    			{
    				TLock lockHead(_head->_lockElem); //блокируем голову от возможного обращения с хвоста
    				_head->_prev=el; //выставляем ссылку на новую голову у старой
    			}
    			_head=el;//выставляем новую голову
    		}
    	}
    
    
    	
    };
    
    
    int main() {
    	TQueue<int> q;
    	q.push(7);
    	q.pop();
    	// your code goes here
    	return 0;
    }

    https://ideone.com/uGX56M
    Ничего не забыл ? Пытался написать очередь для межпоточной синхронизации.

    Запостил: IKing, 02 Июля 2014

    Комментарии (31) RSS

    • Ответить
    • Имхо, data race по переменным _tail и _isEmpty между 45-46 и 60-64 строками. Может быть и еще что-то есть, но я сегодня ленив и туп.
      Ответить
      • https://ideone.com/uGX56M
        Кажется поправил.
        Ответить
        • А теперь дедлок:
          - сразу после создания isEmpty == true, tail == nullptr;
          - первый тред входит в pop, берет lockTail, видит nullptr в tail;
          - второй тред входит в push, берет lockHead, видит true в isEmpty;
          - первый тред ждет lockHead
          - второй тред ждет lockTail
          Ответить
          • https://ideone.com/uGX56M
            Получается разблокировав lockHead до wait я избавлюсь от проблемы?
            Ответить
            • Нет. Чтобы избавиться от проблемы нужно скачать готовую либу с нормальной очередью, в которой все проблемы уже отловили и отладили...

              Вот к примеру: http://www.boost.org/doc/libs/1_55_0/doc/html/lockfree.html
              Ответить
              • Чтобы пользоваться готовой либой нужно понимать как она устроена, какие подводные камни она скрывает. Если сам не напишу очередь понимать этого не буду.
                Ответить
        • cleaned
          Ответить
    • показать все, что скрытоcleaned
      Ответить
    • Могу посоветовать:
      Relacy Race Detector
      Ответить
      • Довольно расистский комментарий.
        Ответить
    • Синхронизированная очередь пишется безо всяких локов, просто через атомный свап.
      Ответить
      • > просто
        Ну-ну.
        Ответить
        • Тарас прав, пишется на CASах, но неправ в том что "просто".
          Хотя что подразумевать под "просто" - придумать и главное доказать корректность изобретенного алгоритма, вряд ли. А реализовать чужое, передрав псевдокод - это сравнительно просто.

          В крестах еще ABA вылазит, в мусоросборных языках с этим чуть проще.
          Ответить
          • Кстати да, я не могу сейчас взять и придумать такой алгоритм (ибо я не помню точно семёнтику доступных функций), просто я его где-то видел, и всё.
            Ответить
      • > безо всяких локов
        > просто

        Если у тебя будет ровно один производитель и ровно один потребитель, то да. И то там блокировку на переполнения кольцевого буфера надо.
        Ответить
        • >И то там блокировку на переполнения кольцевого буфера надо.
          Не надо. Притом что у него связная очередь, а не на буфере.
          http://www.csie.nctu.edu.tw/~tlhuang/Papers/ICPADS00.pdf
          Ответить
          • А точно просто? :)
            То, что можно сложно и без лока, я в курсе.
            Ответить
            • >А точно просто? :)
              Я борманду ответил про "просто". Смотря что - реализовывать чужое или придумывать своё, или вообще "просто" - имелось ввиду без лочек.
              Для связных существует Michael & Scott.
              Ответить
          • Битая ссылка
            Ответить
            • http://people.cs.nctu.edu.tw/~tlhuang/Papers/ICPADS00.pdf

              http://www.computer.org/csdl/proceedings/icpads/2000/0568/00/05680470.pdf
              Ответить
    • О! mutable
      Люди, зачем вам const в языке, если у вас столько способов его снять ? :)
      Ответить
      • лучше бы спросил, зачем mutable, если нет ни одного const - метода
        Ответить
        • например чтоб константный объект менять. Можно ещё и с ссылочками заморочиться для обхода константности.
          Ответить
          • Но в этом коде же нет ни одного const метода, и ни одного метода, принимающего константную ссылку на такой объект, а mutable переменные валяются в привате... Роман именно о конкретном коде, а не об общем случае.
            Ответить
      • показать все, что скрытоКонстанты нужны не для людей. а для оптимизации кода. А если константу можно менять она несет только вред - и людям и коду
        Ответить
      • WSAStringToAddress - вот этот мудак принимает первым аргументом неконстантную строку. const_cast написать короче, чем локальный неконстантный вектор с копией строки :)
        Ответить
    • // your code goes here
      Мой код?
      Ответить

    Добавить комментарий