Pull to refresh

Пять подводных камней при использовании shared_ptr

Reading time 15 min
Views 135K
Класс shared_ptr — это удобный инструмент, который может решить множество проблем разработчика. Однако для того, чтобы не совершать ошибок, необходимо отлично знать его устройство. Надеюсь, моя статья будет полезна тем, кто только начинает работать с этим инструментом.

Я расскажу о следующем:
  • что такое перекрестные ссылки;
  • чем опасны безымянные shared_ptr;
  • какие опасности подстерегают при использовании shared_ptr в многопоточной среде;
  • о чем важно помнить, создавая свою собственную освобождающую функцию для shared_ptr;
  • какие существуют особенности использования шаблона enable_shared_from_this.


Описанные проблемы имеют место как для boost::shared_ptr, так и для std::shared_ptr. В конце статьи вы найдете приложение с полными текстами программ, написанных для демонстрации описываемых особенностей (на примере библиотеки boost).

Перекрестные ссылки


Данная проблема является наиболее известной и связана с тем, что указатель shared_ptr основан на подсчете ссылок. Для экземпляра объекта, которым владеет shared_ptr, создается счетчик. Этот счетчик является общим для всех shared_ptr, указывающих на данный объект.



При конструировании нового объекта создается объект со счетчиком и в него помещается значение 1. При копировании счетчик увеличивается на 1. При вызове деструктора (или при замене указателя путем присваивания, или вызова reset), счетчик уменьшается на 1.

Рассмотрим пример:
struct Widget {
    shared_ptr<Widget> otherWidget;
};

void foo() {
    shared_ptr<Widget> a(new Widget);
    shared_ptr<Widget> b(new Widget);
    a->otherWidget = b;
    // В этой точке у второго объекта счетчик ссылок = 2
    b->otherWidget = a;
    // В этой точке у обоих объектов счетчик ссылок = 2
}

Что произойдет при выходе объектов a и b из области определения? В деструкторе уменьшатся ссылки на объекты. У каждого объекта будет счетчик = 1 (ведь a все еще указывает на b, а b — на a). Объекты “держат” друг друга и у нашего приложения нет возможности получить к ним доступ — эти объекты “потеряны”.
Для решения этой проблемы существует weak_ptr. Одним из типичных случаев создания перекрестных ссылок является случай, когда один объект владеет коллекцией других объектов

struct RootWidget {
    list<shared_ptr<class Widget> > widgets;
};

struct Widget {
    shared_ptr<class RootWidget> parent;
};

При таком устройстве каждый Widget будет препятствовать удалению RootWidget и наоборот.



В таком случае нужно ответить на вопрос: “Кто кем владеет?”. Очевидно, что именно RootWidget в данном случае владеет объектами Widget, а не наоборот. Поэтому модифицировать пример нужно так:

struct Widget {
    weak_ptr<class RootWidget> parent;
};

Слабые ссылки не препятствуют удалению объекта. Они могут быть преобразованы в сильные двумя способами:

1) Конструктор shared_ptr
weak_ptr<Widget> w = …;
// В случае, если объект уже удален, в конструкторе shared_ptr будет сгенерировано исключение
shared_ptr<Widget> p( w );

2) Метод lock
weak_ptr<Widget> w = …;
// В случае, если объект уже удален, то p будет пустым указателем
if( shared_ptr<Widget> p = w.lock() ) {
// Объект не был удален – с ним можно работать
}

Вывод:
В случае возникновения в коде кольцевых ссылок, используйте weak_ptr для решения проблем.

Безымянные указатели


Проблема безымянных указателей относится к вопросу о “точках следования” (sequence points)
// shared_ptr, который передается в функцию foo - безымянный
foo( shared_ptr<Widget>(new Widget), bar() );

// shared_ptr, который передается в функцию foo имеет имя p
shared_ptr<Widget> p(new Widget);
foo( p, bar() );

Из этих двух вариантов документация рекомендует всегда использовать второй — давать указателям имена. Рассмотрим пример, когда функция bar определена вот так:
int bar() {
    throw std::runtime_error(“Exception from bar()”);
}

Дело в том, что в первом случае порядок конструирования не определен. Все зависит от конкретного компилятора и флагов компиляции. Например, это может произойти так:
  1. new Widget
  2. вызов функции bar
  3. конструирование shared_ptr
  4. вызов функции foo

Наверняка можно быть уверенным лишь в том, что вызов foo будет последним действием, а shared_ptr будет сконструирован после создания объекта (new Widget). Однако никаких гарантий того, что он будет сконструирован сразу после создания объекта, нет.

Если во время второго шага будет сгенерировано исключение (а оно в нашем примере сгенерировано будет), то Widget будет считаться сконструированным, но shared_ptr еще не будет им владеть. В итоге ссылка на этот объект будет потеряна. Я проверил данный пример на gcc 4.7.2. Порядок вызова был таким, что shared_ptr+new вне зависимости от опций компиляции не разделялись вызовом bar. Но полагаться именно на такое поведение не стоит – это не гарантировано. Буду признателен, если мне подскажут компилятор, его версию и опции компиляции, для которых подобный код приведет к ошибке.

Еще одна возможность для обхода проблемы анонимных shared_ptr — это использование функций make_shared или allocate_shared. Для нашего примера это будет выглядеть так:

foo( make_shared<Widget>(), bar() );

Данный пример выглядит даже более лаконично, чем исходный, а так же обладает рядом преимуществ в плане выделения памяти (вопросы эффективности оставим за пределами статьи). Допустим вызов make_shared с любым количество аргументов. Например следующий код вернет shared_ptr на строку, созданную через конструктор с одним параметром.

make_shared<string>("shared string");

Вывод:
Давайте shared_ptr имена, даже если код будет от этого менее лаконичным, либо воспользуйтесь для создания объектов
функциями make_shared и allocate_shared.

Проблема использования в разных потоках


Подсчет ссылок в shared_ptr построен с использованием атомарного счетчика. Мы без опаски используем указатели на один и тот же объект из разных потоков. Во всяком случае, мы не привыкли беспокоиться о подсчете ссылок (потокобезопасность самого объекта – другая проблема).

Допустим, у нас есть глобальный shared_ptr:
shared_ptr<Widget> globalSharedPtr(new Widget);

void read() {
    shared_ptr<Widget> x = globalSharedPtr;
    // Сделать что-нибудь с Widget
}

Запустите вызов read из разных потоков и вы увидите, что никаких проблем в коде не возникает (до тех пор, пока вы выполняете над Widget потокобезопасные для этого класса операции).

Допустим, есть еще одна функция:
void write() {
     globalSharedPtr.reset( new Widget );
}

Устройство shared_ptr достаточно сложно, поэтому я приведу код, который схематически поможет показать проблему. Разумеется, настоящий код выглядит иначе.
shared_ptr::shared_ptr(const shared_ptr<T>& x) {
A1:    pointer = x.pointer;
A2:    counter = x.counter;
A3:    atomic_increment( *counter );
}

shared_ptr<T>::reset(T* newObject) {
B1:    if( atomic_decrement( *counter ) == 0 ) {
B2:        delete pointer;
B3:        delete counter;
B4:    }
B5:    pointer = newObject;
B6:    counter = new Counter;
}

Допустим, первый поток начал копировать globalSharedPtr (read), а второй поток вызывает reset для этого же экземпляра указателя (write). В итоге может получиться следующее:
  1. Поток1 только что выполнил строку A2, но еще не перешел к строке A3 (атомарный инкремент).
  2. Поток2 в это время уменьшил счетчик на строке B1, увидел, что после уменьшения счетчик стал равен нулю и выполнил строки B2 и B3.
  3. Поток1 доходит до строки A3 и пытается атомарно увеличить счетчик, которого уже нет.

А может быть и так, что поток1 на строке A2 успеет увеличить счетчик до того, как поток2 вызовет удаление объектов, но после того как поток2 произвел уменьшение счетчика. Тогда мы получим новый shared_ptr, указывающий на удаленный счетчик и объект.

Можно написать подобный код:
shared_ptr<Widget> globalSharedPtr(new Widget);
mutex_t globalSharedPtrMutex;

void resetGlobal(Widget* x) {
    write_lock_t l(globalSharedPtrMutex);
    globalSharedPtr.reset( x );
}

shared_ptr<Widget> getGlobal() {
    read_lock_t l(globalSharedPtrMutex);
    return globalSharedPtr;
}

void read() {
    shared_ptr<Widget> x = getGlobal();
    // Вот с этим x теперь можно работать
}

void write() {
     resetGlobal( new Widget );
}

Теперь, используя такие функции, можно безопасно работать с этим shared_ptr.

Вывод: если какой-то экземпляр shared_ptr доступен разным потокам и может быть модифицирован, то необходимо позаботиться о синхронизации доступа к этому экземпляру shared_ptr.

Особенности времени разрушения освобождающего функтора для shared_ptr


Данная проблема может иметь место только в том случае, если вы используете собственный освобождающий функтор в сочетании со слабыми указателями (weak_ptr). Например, вы можете создать shared_ptr на основе другого shared_ptr, добавив новое действие перед удалением (по сути шаблон “Декоратор”). Так вы могли бы получить указатель для работы с базой данных, изъяв его из пула соединений, а по окончании работы клиента с указателем — вернуть его обратно в пул.
typedef shared_ptr<Connection> ptr_t;

class ConnectionReleaser {
    list<ptr_t>& whereToReturn;
    ptr_t connectionToRelease;
public:
    ConnectionReleaser(list<ptr_t>& lst, const ptr_t& x):whereToReturn(lst), connectionToRelease(x) {}

    void operator()(Connection*) {
        whereToReturn.push_back( connectionToRelease );
// Обратите внимание на следующую строчку
        connectionToRelease.reset();
    }
};

ptr_t getConnection() {
    ptr_t c( connectionList.back() );
    connectionList.pop_back();
    ptr_t r( c.get(), ConnectionReleaser( connectionList, c ) );
    return r; 
}



Проблема заключается в том, что объект, переданный в качестве освобождающего функтора для shared_ptr, будет разрушен только тогда, когда все ссылки на объект будут уничтожены — как сильные(shared_ptr), так и слабые(weak_ptr). Таким образом, если ConnectionReleaser не позаботится о том, чтобы “отпустить” переданный ему указатель (connectionToRelease), он будет держать сильную ссылку, пока существует хотя бы один weak_ptr от shared_ptr, созданного функцией getConnection. Это может привести к достаточно неприятному и неожиданному поведению вашего приложения.

Возможен так же вариант, когда вы воспользуетесь bind для создания освобождающего функтора. Например так:
void releaseConnection(std::list<ptr_t>& whereToReturn, ptr_t& connectionToRelease) {
    whereToReturn.push_back( connectionToRelease );
    // Обратите внимание на следующую строчку
    connectionToRelease.reset();
}

ptr_t getConnection() {
    ptr_t c( connectionList.back() );
    connectionList.pop_back();
    ptr_t r( c.get(), boost::bind(&releaseConnection, boost::ref(connectionList), c) );
    return r;
}


Помните, что bind копирует переданные ему аргументы (кроме случая с использованием boost::ref), и если среди них будет shared_ptr, то его тоже следует очистить, дабы избежать уже описанной проблемы.

Вывод: Выполните в освобождающей функции все действия, которые необходимо совершить при разрушении последней сильной ссылки. Сбросьте все shared_ptr, которые по какой-то причине являются членам вашего функтора. Если вы используете bind, то не забывайте, что он копирует переданные ему аргументы.

Особенности работы с шаблоном enable_shared_from_this


Иногда требуется получить shared_ptr из методов самого объекта. Попытка создания нового shared_ptr от this приведет к неопределенному поведению (скорее всего к аварийному завершению программы), в отличие от intrusive_ptr, для которого это является обычной практикой. Для решения этой проблемы был придуман шаблонный класс-примесь enable_shared_from_this.

Шаблон enable_shared_from_this устроен следующим образом: внутри класса содержится weak_ptr, в который при конструировании shared_ptr помещается ссылка на этот самый shared_ptr. При вызове метода shared_from_this объекта, weak_ptr преобразуется в shared_ptr через конструктор. Схематически шаблон выглядит так:
template<class T> 
class enable_shared_from_this {
    weak_ptr<T> weak_this_;
public:
    shared_ptr<T> shared_from_this() {
        // Преобразование слабой ссылки в сильную через конструктор shared_ptr
        shared_ptr<T> p( weak_this_ );
        return p;
    }
};

class Widget: public enable_shared_from_this<Widget> {};

Конструктор shared_ptr для этого случая схематически выглядит так:
shared_ptr::shared_ptr(T* object) {
    pointer = object;
    counter = new Counter;
    object->weak_this_ = *this;
}

Важно понимать, что при конструировании объекта weak_this_ еще ни на что не указывает. Правильная ссылка в нем появится только после того, как сконструированный объект будет передан в конструктор shared_ptr. Любая попытка вызова shared_from_this из конструктора приведет к bad_weak_ptr исключению.
struct BadWidget: public enable_shared_from_this<BadWidget> {
    BadWidget() {
        // При вызове shared_from_this() будет сгенерировано bad_weak_ptr
         cout << shared_from_this() << endl;
    }
};

К таким же последствиям приведет попытка обратиться к shared_from_this из деструктора, но уже по другой причине: в момент разрушения объекта уже считается, что на него не указывает никаких сильных ссылок (счетчик декрементирован).
struct BadWidget: public enable_shared_from_this<BadWidget> {
    ~BadWidget() {
        // При вызове shared_from_this() будет сгенерировано bad_weak_ptr
        cout << shared_from_this() << endl;
    }
};

Со вторым случаем (деструктор) мало что можно придумать. Единственный вариант — позаботиться о том, чтобы не вызывать shared_from_this и сделать так, чтобы этого не делали функции, которые вызывает деструктор.

С первым случаем все обстоит немного проще. Наверняка вы уже решили, что единственный способ для существования вашего объекта — shared_ptr, тогда будет уместно переместить конструктор объекта в закрытую часть класса и создать статический метод для создания shared_ptr нужного вам типа. Если при инициализации объекта вам нужно выполнить действия, которым потребуется shared_from_this, то для этой цели можно выделить логику в метод init.

class GoodWidget: public enable_shared_from_this<GoodWidget> {
        void init() {
                cout << shared_from_this() << endl;
        }
public:
        static shared_ptr<GoodWidget> create() {
                shared_ptr<GoodWidget> p(new GoodWidget);
                p->init();
                return p;
        }
};

Вывод:
Избегайте вызовов (прямых или косвенных) shared_from_this из конструкторов и деструкторов. В случае, если для правильной инициализации объекта требуется доступ к shared_from_this: создайте метод init, делегируйте создание объекта статическому методу и сделайте так, чтобы объекты можно было создавать только с помощью этого метода.

Заключение


В статье рассмотрены 5 особенностей использования shared_ptr и представлены общие рекомендации по избежанию потенциальных проблем.

Хотя shared_ptr и снимает с разработчика множество проблем, знание внутреннего устройства (хоть и примерно) является обязательным для грамотного использования shared_ptr. Я рекомендую внимательно изучить устройство shared_ptr, а так же классов, связанных с ним. Соблюдение ряда простых правил может уберечь разработчика от нежелательных проблем.

Литература




Приложение


В приложении представлены полные тексты программ для иллюстрации описанных в статье случаев

Демонстрация проблемы кольцевых ссылок
#include <string>
#include <iostream>
#include <boost/shared_ptr.hpp>
#include <boost/weak_ptr.hpp>

class BadWidget {
	std::string name;
	boost::shared_ptr<BadWidget> otherWidget;
public:
	BadWidget(const std::string& n):name(n) {
		std::cout << "BadWidget " << name << std::endl;
	}

	~BadWidget() {
		std::cout << "~BadWidget " << name << std::endl;
	}

	void setOther(const boost::shared_ptr<BadWidget>& x) {
		otherWidget = x;
		std::cout << name << " now points to " << x->name << std::endl;
	}
};

class GoodWidget {
	std::string name;
	boost::weak_ptr<GoodWidget> otherWidget;
public:
	GoodWidget(const std::string& n):name(n) {
		std::cout << "GoodWidget " << name << std::endl;
	}

	~GoodWidget() {
		std::cout << "~GoodWidget " << name << std::endl;
	}

	void setOther(const boost::shared_ptr<GoodWidget>& x) {
		otherWidget = x;
		std::cout << name << " now points to " << x->name << std::endl;
	}
};

int main() {
	{ // В этом примере происходит утечка памяти
		std::cout << "====== Example 3" << std::endl;
		boost::shared_ptr<BadWidget> w1(new BadWidget("3_First"));
		boost::shared_ptr<BadWidget> w2(new BadWidget("3_Second"));
		w1->setOther( w2 );
		w2->setOther( w1 );
	}
	{ // А в этом примере использован weak_ptr и утечки памяти не происходит
		std::cout << "====== Example 3" << std::endl;
		boost::shared_ptr<GoodWidget> w1(new GoodWidget("4_First"));
		boost::shared_ptr<GoodWidget> w2(new GoodWidget("4_Second"));
		w1->setOther( w2 );
		w2->setOther( w1 );
	}
	return 0;
}

Демонстрация преобразования weak_ptr в shared_ptr
#include <iostream>
#include <boost/shared_ptr.hpp>
#include <boost/weak_ptr.hpp>

class Widget {};

int main() {
	boost::weak_ptr<Widget> w;
	// В этой точке weak_ptr ни на что не указывает
	// Метод lock вернет пустой указатель
	std::cout << __LINE__ << ": " << w.lock().get() << std::endl;
	// Конструирование shared_ptr от этого указателя приведет к исключению
	try {
		boost::shared_ptr<Widget> tmp ( w );
	} catch (const boost::bad_weak_ptr&) {
		std::cout << __LINE__ << ": bad_weak_ptr" << std::endl;
	}

	boost::shared_ptr<Widget> p(new Widget);
	// Теперь у weak_ptr есть значение
	w = p;

	// Метод lock вернет правильный указатель
	std::cout << __LINE__ << ": " << w.lock().get() << std::endl;
	// Конструирование shared_ptr от этого указателя тоже вернет правильный указатель. Исключения не будет
	std::cout << __LINE__ << ": " << boost::shared_ptr<Widget>( w ).get() << std::endl;

	// Сбросим указатель
	p.reset();
	// Сильных ссылок больше нет. У weak_ptr истек срок годности

	// Метод lock снова вернет пустой указатель
	std::cout << __LINE__ << ": " << w.lock().get() << std::endl;
	// Конструирование shared_ptr от этого указателя снова приведет к исключению
	try {
		boost::shared_ptr<Widget> tmp ( w );
	} catch (const boost::bad_weak_ptr&) {
		std::cout << __LINE__ << ": bad_weak_ptr" << std::endl;
	}
	return 0;
}

Демонстрация проблемы многопоточности для shared_ptr
#include <iostream>

#include <boost/thread.hpp>
#include <boost/shared_ptr.hpp>

typedef boost::shared_mutex mutex_t;
typedef boost::unique_lock<mutex_t> read_lock_t;
typedef boost::shared_lock<mutex_t> write_lock_t;

mutex_t globalMutex;
boost::shared_ptr<int> globalPtr(new int(0));

const int readThreads = 10;
const int maxOperations = 10000;

boost::shared_ptr<int> getPtr() {
// Закомментируйте следующую строку, чтобы ваше приложение упало
	read_lock_t l(globalMutex);
	return globalPtr;
}

void resetPtr(const boost::shared_ptr<int>& x) {
// Закомментируйте следующую строку, чтобы ваше приложение упало
	write_lock_t l(globalMutex);
	globalPtr = x;
}

void myRead() {
	for(int i = 0; i < maxOperations; ++i) {
    	boost::shared_ptr<int> p = getPtr();
	}
}

void myWrite() {
	for(int i = 0; i < maxOperations; ++i) {
		resetPtr( boost::shared_ptr<int>( new int(i)) );
	}
}

int main() {
	boost::thread_group tg;
	tg.create_thread( &myWrite );
	for(int i = 0; i < readThreads; ++i) {
		tg.create_thread( &myRead  );
	}
	tg.join_all();
	return 0;
}


Демонстрация проблемы deleter + weak_ptr
#include <string>
#include <list>
#include <iostream>
#include <stdexcept>

#include <boost/shared_ptr.hpp>
#include <boost/weak_ptr.hpp>
#include <boost/bind.hpp>

class Connection {
	std::string name;
public:
	const std::string& getName() const { return name; }

	explicit Connection(const std::string& n):name(n) {
		std::cout << "Connection " << name << std::endl;
	}

	~Connection() {
		std::cout << "~Connection " << name << std::endl;
	}
};

typedef boost::shared_ptr<Connection> ptr_t;

class ConnectionPool {
	std::list<ptr_t> connections;

	// Этот класс предназначен для демонстрации первого варианта создания deleter (get1)
	class ConnectionReleaser {
		std::list<ptr_t>& whereToReturn;
		ptr_t connectionToRelease;
	public:
		ConnectionReleaser(std::list<ptr_t>& lst, const ptr_t& x):whereToReturn(lst), connectionToRelease(x) {}

		void operator()(Connection*) {
			whereToReturn.push_back( connectionToRelease );
			std::cout << "get1: Returned connection " << connectionToRelease->getName() << " to the list" << std::endl;

			// Закомментируйте след. строку и обратите внимание на разницу в выходной печати
			connectionToRelease.reset();
		}
	};

	// Эта функция предназначена для демонстрации второго варианта создания deleter (get2)
	static void releaseConnection(std::list<ptr_t>& whereToReturn, ptr_t& connectionToRelease) {
		whereToReturn.push_back( connectionToRelease );
		std::cout << "get2: Returned connection " << connectionToRelease->getName() << " to the list" << std::endl;

		// Закомментируйте следующую строку и обратите внимание на разницу в выходной печати
		connectionToRelease.reset();
	}

	ptr_t popConnection() {
		if( connections.empty() ) throw std::runtime_error("No connections left");
		ptr_t w( connections.back() );
		connections.pop_back();
		return w;
	}
public:
	ptr_t get1() {
		ptr_t w = popConnection();
		std::cout << "get1: Taken connection " << w->getName() << " from list" << std::endl;
		ptr_t r( w.get(), ConnectionReleaser( connections, w ) );
		return r;
	}

	ptr_t get2() {
		ptr_t w = popConnection();
		std::cout << "get2: Taken connection " << w->getName() << " from list" << std::endl;
		ptr_t r( w.get(), boost::bind(&releaseConnection, boost::ref(connections), w ));
		return r;
	}

	void add(const std::string& name) {
		connections.push_back( ptr_t(new Connection(name)) );
	}

	ConnectionPool() {
		std::cout << "ConnectionPool" << std::endl;
	}

	~ConnectionPool() {
		std::cout << "~ConnectionPool" << std::endl;
	}
};

int main() {
	boost::weak_ptr<Connection> weak1;
	boost::weak_ptr<Connection> weak2;
	{
		ConnectionPool cp;
		cp.add("One");
		cp.add("Two");

		ptr_t p1 = cp.get1();
		weak1 = p1;
		ptr_t p2 = cp.get2();
		weak2 = p2;
	}
	std::cout << "Here the ConnectionPool is out of scope, but weak_ptrs are not" << std::endl;
	return 0;
}

Демонстрация проблемы с enable_shared_from_this
#include <iostream>
#include <boost/shared_ptr.hpp>
#include <boost/enable_shared_from_this.hpp>

class BadWidget1: public boost::enable_shared_from_this<BadWidget1> {
public:
	BadWidget1() {
		std::cout << "Constructor" << std::endl;
		std::cout << shared_from_this() << std::endl;
	}
};

class BadWidget2: public boost::enable_shared_from_this<BadWidget2> {
public:
	~BadWidget2() {
		std::cout << "Destructor" << std::endl;
		std::cout << shared_from_this() << std::endl;
	}
};

class GoodWidget: public boost::enable_shared_from_this<GoodWidget> {
	GoodWidget() {}

	void init() {
		std::cout << "init()" << std::endl;
		std::cout << shared_from_this() << std::endl;
	}
public:
	static boost::shared_ptr<GoodWidget> create() {
		boost::shared_ptr<GoodWidget> p(new GoodWidget);
		p->init();
		return p;
	}
};


int main() {
	boost::shared_ptr<GoodWidget> good = GoodWidget::create();
	try {
		boost::shared_ptr<BadWidget1> bad1(new BadWidget1);
	} catch( const boost::bad_weak_ptr&) {
		std::cout << "Caught bad_weak_ptr for BadWidget1" << std::endl;
	}
	try {
		boost::shared_ptr<BadWidget2> bad2(new BadWidget2);
		// При компиляции с новым стандартом вы получите terminate
		// т.к. считается, что из деструктора по умолчанию не может быть сгенерировано исключение
	} catch( const boost::bad_weak_ptr&) {
		std::cout << "Caught bad_weak_ptr for BadWidget2" << std::endl;
	}
	return 0;
}
Tags:
Hubs:
+55
Comments 53
Comments Comments 53

Articles