SplitterChannel::addChannel() should only add a channel once

SplitterChannel::addChannel() should only add a channel once to the internal vector. This prevents issues where the channel is accidentally added twice but only removed once because removeChannel stops at the first result. (#4270)

Foundation/src/SplitterChannel.cpp

@@ -43,21 +43,24 @@ void SplitterChannel::addChannel(Channel::Ptr pChannel)
 	poco_check_ptr(pChannel);
 
 	FastMutex::ScopedLock lock(_mutex);
+
+	// ensure that the channel is only added once
+	if (std::find(_channels.begin(), _channels.end(), pChannel) == _channels.end())
+	{
 		_channels.push_back(pChannel);
 	}
+}
 
 
 void SplitterChannel::removeChannel(Channel::Ptr pChannel)
 {
 	FastMutex::ScopedLock lock(_mutex);
 
-	for (ChannelVec::iterator it = _channels.begin(); it != _channels.end(); ++it)
+	const auto it = std::find(_channels.begin(), _channels.end(), pChannel);
-	{
+
-		if (*it == pChannel)
+	if (it != _channels.end())
 	{
 		_channels.erase(it);
-			break;
-		}
 	}
 }
 

Foundation/testsuite/src/ChannelTest.cpp

@@ -84,16 +84,34 @@ ChannelTest::~ChannelTest()
 
 
 void ChannelTest::testSplitter()
+{
+	AutoPtr<TestChannel> pChannel1 = new TestChannel;
+	AutoPtr<TestChannel> pChannel2 = new TestChannel;
+	AutoPtr<SplitterChannel> pSplitter = new SplitterChannel;
+	pSplitter->addChannel(pChannel1);
+	pSplitter->addChannel(pChannel2);
+	Message msg;
+	pSplitter->log(msg);
+	assertTrue(pChannel1->list().size() == 1);
+	assertTrue(pChannel2->list().size() == 1);
+}
+
+void ChannelTest::testSplitterAddSameChannelTwice()
 {
 	AutoPtr<TestChannel> pChannel = new TestChannel;
 	AutoPtr<SplitterChannel> pSplitter = new SplitterChannel;
 	pSplitter->addChannel(pChannel);
 	pSplitter->addChannel(pChannel);
+
+	assertTrue(pSplitter->count() == 1);
+
 	Message msg;
 	pSplitter->log(msg);
-	assertTrue (pChannel->list().size() == 2);
-}
 
+	pSplitter->removeChannel(pChannel);
+
+	assertTrue(pSplitter->count() == 0);
+}
 
 void ChannelTest::testAsync()
 {
@@ -162,6 +180,7 @@ CppUnit::Test* ChannelTest::suite()
 	CppUnit::TestSuite* pSuite = new CppUnit::TestSuite("ChannelTest");
 
 	CppUnit_addTest(pSuite, ChannelTest, testSplitter);
+	CppUnit_addTest(pSuite, ChannelTest, testSplitterAddSameChannelTwice);
 	CppUnit_addTest(pSuite, ChannelTest, testAsync);
 	CppUnit_addTest(pSuite, ChannelTest, testFormatting);
 	CppUnit_addTest(pSuite, ChannelTest, testConsole);

Foundation/testsuite/src/ChannelTest.h

@@ -25,6 +25,7 @@ public:
 	~ChannelTest();
 
 	void testSplitter();
+	void testSplitterAddSameChannelTwice();
 	void testAsync();
 	void testFormatting();
 	void testConsole();