Result of svn merge -r59958:60105 svn+ssh://svn/svn/linden/branches/discard-garbage into release

4 files changed, 388 insertions, 45 deletions

diff --git a/indra/llmessage/llbuffer.cpp b/indra/llmessage/llbuffer.cpp
index e4200b914b..6b0c4f8fab 100644
--- a/indra/llmessage/llbuffer.cpp
+++ b/indra/llmessage/llbuffer.cpp
@@ -65,24 +65,44 @@ S32 LLSegment::size() const
 	return mSize;
 }
 
+bool LLSegment::operator==(const LLSegment& rhs) const
+{
+	if((mData != rhs.mData)||(mSize != rhs.mSize)||(mChannel != rhs.mChannel))
+	{
+		return false;
+	}
+	return true;
+}
 
 /** 
  * LLHeapBuffer
  */
-LLHeapBuffer::LLHeapBuffer() 
+LLHeapBuffer::LLHeapBuffer() :
+	mBuffer(NULL),
+	mSize(0),
+	mNextFree(NULL),
+	mReclaimedBytes(0)
 {
 	LLMemType m1(LLMemType::MTYPE_IO_BUFFER);
 	const S32 DEFAULT_HEAP_BUFFER_SIZE = 16384;
 	allocate(DEFAULT_HEAP_BUFFER_SIZE);
 }
 
-LLHeapBuffer::LLHeapBuffer(S32 size)
+LLHeapBuffer::LLHeapBuffer(S32 size) :
+	mBuffer(NULL),
+	mSize(0),
+	mNextFree(NULL),
+	mReclaimedBytes(0)
 {
 	LLMemType m1(LLMemType::MTYPE_IO_BUFFER);
 	allocate(size);
 }
 
-LLHeapBuffer::LLHeapBuffer(const U8* src, S32 len)
+LLHeapBuffer::LLHeapBuffer(const U8* src, S32 len) :
+	mBuffer(NULL),
+	mSize(0),
+	mNextFree(NULL),
+	mReclaimedBytes(0)
 {
 	LLMemType m1(LLMemType::MTYPE_IO_BUFFER);
 	if((len > 0) && src)
@@ -93,12 +113,6 @@ LLHeapBuffer::LLHeapBuffer(const U8* src, S32 len)
 			memcpy(mBuffer, src, len);	/*Flawfinder: ignore*/
 		}
 	}
-	else
-	{
-		mBuffer = NULL;
-		mSize = 0;
-		mNextFree = NULL;
-	}
 }
 
 // virtual
@@ -111,11 +125,10 @@ LLHeapBuffer::~LLHeapBuffer()
 	mNextFree = NULL;
 }
 
-// virtual
-//S32 LLHeapBuffer::bytesLeft() const
-//{
-//	return (mSize - (mNextFree - mBuffer));
-//}
+S32 LLHeapBuffer::bytesLeft() const
+{
+	return (mSize - (mNextFree - mBuffer));
+}
 
 // virtual
 bool LLHeapBuffer::createSegment(
@@ -139,9 +152,47 @@ bool LLHeapBuffer::createSegment(
 	return true;
 }
 
+// virtual
+bool LLHeapBuffer::reclaimSegment(const LLSegment& segment)
+{
+	if(containsSegment(segment))
+	{
+		mReclaimedBytes += segment.size();
+		if(mReclaimedBytes == mSize)
+		{
+			// We have reclaimed all of the memory from this
+			// buffer. Therefore, we can reset the mNextFree to the
+			// start of the buffer, and reset the reclaimed bytes.
+			mReclaimedBytes = 0;
+			mNextFree = mBuffer;
+		}
+		else if(mReclaimedBytes > mSize)
+		{
+			llwarns << "LLHeapBuffer reclaimed more memory than allocated."
+				<< " This is probably programmer error." << llendl;
+		}
+		return true;
+	}
+	return false;
+}
+
+// virtual
+bool LLHeapBuffer::containsSegment(const LLSegment& segment) const
+{
+	// *NOTE: this check is fairly simple because heap buffers are
+	// simple contiguous chunks of heap memory.
+	if((mBuffer > segment.data())
+	   || ((mBuffer + mSize) < (segment.data() + segment.size())))
+	{
+		return false;
+	}
+	return true;
+}
+
 void LLHeapBuffer::allocate(S32 size)
 {
 	LLMemType m1(LLMemType::MTYPE_IO_BUFFER);
+	mReclaimedBytes = 0;	
 	mBuffer = new U8[size];
 	if(mBuffer)
 	{
@@ -180,6 +231,18 @@ LLChannelDescriptors LLBufferArray::nextChannel()
 	return rv;
 }
 
+S32 LLBufferArray::capacity() const
+{
+	S32 total = 0;
+	const_buffer_iterator_t iter = mBuffers.begin();
+	const_buffer_iterator_t end = mBuffers.end();
+	for(; iter != end; ++iter)
+	{
+		total += (*iter)->capacity();
+	}
+	return total;
+}
+
 bool LLBufferArray::append(S32 channel, const U8* src, S32 len)
 {
 	LLMemType m1(LLMemType::MTYPE_IO_BUFFER);
@@ -684,14 +747,31 @@ LLBufferArray::segment_iterator_t LLBufferArray::makeSegment(
 	return send;
 }
 
-bool LLBufferArray::eraseSegment(const segment_iterator_t& iter)
+bool LLBufferArray::eraseSegment(const segment_iterator_t& erase_iter)
 {
 	LLMemType m1(LLMemType::MTYPE_IO_BUFFER);
-	// *FIX: in theory, we could reclaim the memory. We are leaking a
-	// bit of buffered memory into an unusable but still referenced
-	// location.
-	(void)mSegments.erase(iter);
-	return true;
+
+	// Find out which buffer contains the segment, and if it is found,
+	// ask it to reclaim the memory.
+	bool rv = false;
+	LLSegment segment(*erase_iter);
+	buffer_iterator_t iter = mBuffers.begin();
+	buffer_iterator_t end = mBuffers.end();
+	for(; iter != end; ++iter)
+	{
+		// We can safely call reclaimSegment on every buffer, and once
+		// it returns true, the segment was found.
+		if((*iter)->reclaimSegment(segment))
+		{
+			rv = true;
+			break;
+		}
+	}
+
+	// No need to get the return value since we are not interested in
+	// the interator retured by the call.
+	(void)mSegments.erase(erase_iter);
+	return rv;
 }
 
 
diff --git a/indra/llmessage/llbuffer.h b/indra/llmessage/llbuffer.h
index 3d7f209123..0a0a457d56 100644
--- a/indra/llmessage/llbuffer.h
+++ b/indra/llmessage/llbuffer.h
@@ -13,7 +13,7 @@
 
 /**
  * Declaration of classes used for minimizing calls to new[],
- * memcpy(), and delete[]. Typically, you would create an LLHeapArray,
+ * memcpy(), and delete[]. Typically, you would create an LLBufferArray,
  * feed it data, modify and add segments as you process it, and feed
  * it to a sink.
  */
@@ -89,6 +89,16 @@ public:
 	 */
 	S32 size() const;
 
+	/** 
+	 * @brief Check if two segments are the same.
+	 *
+	 * Two segments are considered equal if they are on the same
+	 * channel and cover the exact same address range.
+	 * @param rhs the segment to compare with this segment.
+	 * @return Returns true if they are equal.
+	 */
+	bool operator==(const LLSegment& rhs) const;
+
 protected:
 	S32 mChannel;
 	U8* mData;
@@ -126,6 +136,35 @@ public:
 	 * @return Returns true if a segment was found.
 	 */
 	virtual bool createSegment(S32 channel, S32 size, LLSegment& segment) = 0;
+
+	/** 
+	 * @brief Reclaim a segment from this buffer. 
+	 *
+	 * This method is called on a buffer object when a caller is done
+	 * with a contiguous segment of memory inside this buffer. Since
+	 * segments can be cut arbitrarily outside of the control of the
+	 * buffer, this segment may not match any segment returned from
+	 * <code>createSegment()</code>.  
+	 * @param segment The contiguous buffer segment to reclaim.
+	 * @return Returns true if the call was successful.
+	 */
+	virtual bool reclaimSegment(const LLSegment& segment) = 0;
+
+	/** 
+	 * @brief Test if a segment is inside this buffer.
+	 *
+	 * @param segment The contiguous buffer segment to test.
+	 * @return Returns true if the segment is in the bufffer.
+	 */
+	virtual bool containsSegment(const LLSegment& segment) const = 0;
+
+	/** 
+	 * @brief Return the current number of bytes allocated.
+	 *
+	 * This was implemented as a debugging tool, and it is not
+	 * necessarily a good idea to use it for anything else.
+	 */
+	virtual S32 capacity() const = 0;
 };
 
 /** 
@@ -167,9 +206,11 @@ public:
 	/** 
 	 * @brief Get the number of bytes left in the buffer.
 	 *
+	 * Note that this is not a virtual function, and only available in
+	 * the LLHeapBuffer as a debugging aid.
 	 * @return Returns the number of bytes left.
 	 */
-	//virtual S32 bytesLeft() const;
+	S32 bytesLeft() const;
 
 	/** 
 	 * @brief Generate a segment for this buffer.
@@ -186,10 +227,40 @@ public:
 	 */
 	virtual bool createSegment(S32 channel, S32 size, LLSegment& segment);
 
+	/** 
+	 * @brief reclaim a segment from this buffer. 
+	 *
+	 * This method is called on a buffer object when a caller is done
+	 * with a contiguous segment of memory inside this buffer. Since
+	 * segments can be cut arbitrarily outside of the control of the
+	 * buffer, this segment may not match any segment returned from
+	 * <code>createSegment()</code>.  
+	 * This call will fail if the segment passed in is note completely
+	 * inside the buffer, eg, if the segment starts before this buffer
+	 * in memory or ends after it.
+	 * @param segment The contiguous buffer segment to reclaim.
+	 * @return Returns true if the call was successful.
+	 */
+	virtual bool reclaimSegment(const LLSegment& segment);
+
+	/** 
+	 * @brief Test if a segment is inside this buffer.
+	 *
+	 * @param segment The contiguous buffer segment to test.
+	 * @return Returns true if the segment is in the bufffer.
+	 */
+	virtual bool containsSegment(const LLSegment& segment) const;
+
+	/** 
+	 * @brief Return the current number of bytes allocated.
+	 */
+	virtual S32 capacity() const { return mSize; }
+
 protected:
 	U8* mBuffer;
 	S32 mSize;
 	U8* mNextFree;
+	S32 mReclaimedBytes;
 
 private:
 	/** 
@@ -204,13 +275,14 @@ private:
  * @brief Class to represent scattered memory buffers and in-order segments
  * of that buffered data.
  *
- * NOTE: This class needs to have an iovec interface
+ * *NOTE: This class needs to have an iovec interface
  */
 class LLBufferArray
 {
 public:
 	typedef std::vector<LLBuffer*> buffer_list_t;
 	typedef buffer_list_t::iterator buffer_iterator_t;
+	typedef buffer_list_t::const_iterator const_buffer_iterator_t;
 	typedef std::list<LLSegment> segment_list_t;
 	typedef segment_list_t::const_iterator const_segment_iterator_t;
 	typedef segment_list_t::iterator segment_iterator_t;
@@ -241,11 +313,16 @@ public:
 	 */
 	LLChannelDescriptors nextChannel();
 	//@}
-	
+
 	/* @name Data methods
 	 */
 	//@{
 
+	/** 
+	 * @brief Return the sum of all allocated bytes.
+	 */
+	S32 capacity() const;
+
 	// These methods will be useful once there is any kind of buffer
 	// besides a heap buffer.
 	//bool append(EBufferChannel channel, LLBuffer* data);
@@ -275,7 +352,6 @@ public:
 	 * new segment is created and put in the front of the array. This
 	 * object will internally allocate new buffers if necessary.
 	 * @param channel The channel for this data
-
 	 * @param src The start of memory for the data to be copied
 	 * @param len The number of bytes of data to copy
 	 * @return Returns true if the method worked.
@@ -359,7 +435,7 @@ public:
 	bool takeContents(LLBufferArray& source);
 	//@}
 
-	/* @name Segment methods 
+	/* @name Segment methods
 	 */
 	//@{
 	/** 
@@ -449,7 +525,7 @@ public:
 	 * endSegment() on failure.
 	 */
 	segment_iterator_t makeSegment(S32 channel, S32 length);
-		
+
 	/** 
 	 * @brief Erase the segment if it is in the buffer array.
 	 *
diff --git a/indra/llmessage/llbufferstream.cpp b/indra/llmessage/llbufferstream.cpp
index 684548b408..e86620c438 100644
--- a/indra/llmessage/llbufferstream.cpp
+++ b/indra/llmessage/llbufferstream.cpp
@@ -43,34 +43,64 @@ int LLBufferStreamBuf::underflow()
 	{
 		return EOF;
 	}
-	LLSegment segment;
-	LLBufferArray::segment_iterator_t it;
-	U8* last_pos = (U8*)gptr();
-	if(last_pos) --last_pos;
-	
-	LLBufferArray::segment_iterator_t end = mBuffer->endSegment();
 
-	// Get iterator to full segment containing last_pos
-	// and construct sub-segment starting at last_pos.
-	// Note: segment may != *it at this point
-	it = mBuffer->constructSegmentAfter(last_pos, segment);
-	if(it == end)
+	LLBufferArray::segment_iterator_t iter;
+	LLBufferArray::segment_iterator_t end = mBuffer->endSegment();
+	U8* last_pos = (U8*)gptr();
+	LLSegment segment;
+	if(last_pos)
+	{
+		// Back up into a piece of memory we know that we have
+		// allocated so that calls for the next segment based on
+		// 'after' will succeed.
+		--last_pos;
+		iter = mBuffer->splitAfter(last_pos);
+		if(iter != end)
+		{
+			// We need to clear the read segment just in case we have
+			// an early exit in the function and never collect the
+			// next segment. Calling eraseSegment() with the same
+			// segment twice is just like double deleting -- nothing
+			// good comes from it.
+			mBuffer->eraseSegment(iter++);
+			if(iter != end) segment = (*iter);
+		}
+		else
+		{
+			// This should never really happen, but somehow, the
+			// istream is telling the buf that it just finished
+			// reading memory that is not in the buf. I think this
+			// would only happen if there were a bug in the c++ stream
+			// class. Just bail.
+			// *TODO: can we set the fail bit on the stream somehow?
+			return EOF;
+		}
+	}
+	else
+	{
+		// Get iterator to full segment containing last_pos
+		// and construct sub-segment starting at last_pos.
+		// Note: segment may != *it at this point
+		iter = mBuffer->constructSegmentAfter(last_pos, segment);
+	}
+	if(iter == end)
 	{
 		return EOF;
 	}
-	
+
 	// Iterate through segments to find a non-empty segment on input channel.
 	while((!segment.isOnChannel(mChannels.in()) || (segment.size() == 0)))
 	{
-		++it;
-		if(it == end)
+		++iter;
+		if(iter == end)
 		{
 			return EOF;
 		}
 
-		segment = *it;
+		segment = *(iter);
 	}
-	
+
+	// set up the stream to read from the next segment.
 	char* start = (char*)segment.data();
 	setg(start, start, start + segment.size());
 	return *gptr();
@@ -125,12 +155,43 @@ int LLBufferStreamBuf::sync()
 		return return_value;
 	}
 
+	// This chunk of code is not necessary because typically, users of
+	// the stream will read until EOF. Therefore, underflow was called
+	// and the segment was discarded before the sync() was called in
+	// the destructor. Theoretically, we could keep some more data
+	// around and detect the rare case where an istream was deleted
+	// before reading to the end, but that will only leave behind some
+	// unavailable but still referenced memory. Also, if another
+	// istream is constructed, it will re-read that segment, and then
+	// discard it.
+	//U8* last_pos = (U8*)gptr();
+	//if(last_pos)
+	//{
+	//	// Looks like we read something. Discard what we have read.
+	//	// gptr() actually returns the currrent position, but we call
+	//	// it last_pos because of how it is used in the split call
+	//	// below.
+	//	--last_pos;
+	//	LLBufferArray::segment_iterator_t iter;
+	//	iter = mBuffer->splitAfter(last_pos);
+	//	if(iter != mBuffer->endSegment())
+	//	{
+	//		// We need to clear the read segment just in case we have
+	//		// an early exit in the function and never collect the
+	//		// next segment. Calling eraseSegment() with the same
+	//		// segment twice is just like double deleting -- nothing
+	//		// good comes from it.
+	//		mBuffer->eraseSegment(iter);
+	//	}
+	//}
+
 	// set the put pointer so that we force an overflow on the next
 	// write.
 	U8* address = (U8*)pptr();
 	setp(NULL, NULL);
 
-	// *NOTE: I bet we could just --address. Need to think about that.
+	// *NOTE: I bet we could just --address if address is not NULL.
+	// Need to think about that.
 	address = mBuffer->seek(mChannels.out(), address, -1);
 	if(address)
 	{
diff --git a/indra/test/io.cpp b/indra/test/io.cpp
index 4908653f0f..034472fa0b 100644
--- a/indra/test/io.cpp
+++ b/indra/test/io.cpp
@@ -31,6 +31,101 @@
 
 namespace tut
 {
+	struct heap_buffer_data
+	{
+		heap_buffer_data() : mBuffer(NULL) {}
+		~heap_buffer_data() { if(mBuffer) delete mBuffer; }
+		LLHeapBuffer* mBuffer;
+	};
+	typedef test_group<heap_buffer_data> heap_buffer_test;
+	typedef heap_buffer_test::object heap_buffer_object;
+	tut::heap_buffer_test thb("heap_buffer");
+
+	template<> template<>
+	void heap_buffer_object::test<1>()
+	{
+		const S32 BUF_SIZE = 100;
+		mBuffer = new LLHeapBuffer(BUF_SIZE);
+		ensure_equals("empty buffer capacity", mBuffer->capacity(), BUF_SIZE);
+		const S32 SEGMENT_SIZE = 50;
+		LLSegment segment;
+		mBuffer->createSegment(0, SEGMENT_SIZE, segment);
+		ensure_equals("used buffer capacity", mBuffer->capacity(), BUF_SIZE);
+	}
+
+	template<> template<>
+	void heap_buffer_object::test<2>()
+	{
+		const S32 BUF_SIZE = 10;
+		mBuffer = new LLHeapBuffer(BUF_SIZE);
+		LLSegment segment;
+		mBuffer->createSegment(0, BUF_SIZE, segment);
+		ensure("segment is in buffer", mBuffer->containsSegment(segment));
+		ensure_equals("buffer consumed", mBuffer->bytesLeft(), 0);
+		bool  created;
+		created = mBuffer->createSegment(0, 0, segment);
+		ensure("Create zero size segment fails", !created);
+		created = mBuffer->createSegment(0, BUF_SIZE, segment);
+		ensure("Create segment fails", !created);
+	}
+
+	template<> template<>
+	void heap_buffer_object::test<3>()
+	{
+		const S32 BUF_SIZE = 10;
+		mBuffer = new LLHeapBuffer(BUF_SIZE);
+		LLSegment segment;
+		mBuffer->createSegment(0, BUF_SIZE, segment);
+		ensure("segment is in buffer", mBuffer->containsSegment(segment));
+		ensure_equals("buffer consumed", mBuffer->bytesLeft(), 0);
+		bool reclaimed = mBuffer->reclaimSegment(segment);
+		ensure("buffer reclaimed.", reclaimed);
+		ensure_equals("buffer available", mBuffer->bytesLeft(), BUF_SIZE);
+		bool  created;
+		created = mBuffer->createSegment(0, 0, segment);
+		ensure("Create zero size segment fails", !created);
+		created = mBuffer->createSegment(0, BUF_SIZE, segment);
+		ensure("Create another segment succeeds", created);
+	}
+
+	template<> template<>
+	void heap_buffer_object::test<4>()
+	{
+		const S32 BUF_SIZE = 10;
+		const S32 SEGMENT_SIZE = 4;
+		mBuffer = new LLHeapBuffer(BUF_SIZE);
+		LLSegment seg1;
+		mBuffer->createSegment(0, SEGMENT_SIZE, seg1);
+		ensure("segment is in buffer", mBuffer->containsSegment(seg1));
+		LLSegment seg2;
+		mBuffer->createSegment(0, SEGMENT_SIZE, seg2);
+		ensure("segment is in buffer", mBuffer->containsSegment(seg2));
+		LLSegment seg3;
+		mBuffer->createSegment(0, SEGMENT_SIZE, seg3);
+		ensure("segment is in buffer", mBuffer->containsSegment(seg3));
+		ensure_equals("segment is truncated", seg3.size(), 2);
+		LLSegment seg4;
+		bool created;
+		created = mBuffer->createSegment(0, SEGMENT_SIZE, seg4);
+		ensure("Create segment fails", !created);
+		bool reclaimed;
+		reclaimed = mBuffer->reclaimSegment(seg1);
+		ensure("buffer reclaim succeed.", reclaimed);
+		ensure_equals("no buffer available", mBuffer->bytesLeft(), 0);
+		reclaimed = mBuffer->reclaimSegment(seg2);
+		ensure("buffer reclaim succeed.", reclaimed);
+		ensure_equals("buffer reclaimed", mBuffer->bytesLeft(), 0);
+		reclaimed = mBuffer->reclaimSegment(seg3);
+		ensure("buffer reclaim succeed.", reclaimed);
+		ensure_equals("buffer reclaimed", mBuffer->bytesLeft(), BUF_SIZE);
+		created = mBuffer->createSegment(0, SEGMENT_SIZE, seg1);
+		ensure("segment is in buffer", mBuffer->containsSegment(seg1));
+		ensure("Create segment succeds", created);
+	}
+}
+
+namespace tut
+{
 	struct buffer_data
 	{
 		LLBufferArray mBuffer;
@@ -209,6 +304,37 @@ namespace tut
 		delete[] temp;
 	}
 
+	template<> template<>
+	void buffer_object::test<9>()
+	{
+		LLChannelDescriptors ch = mBuffer.nextChannel();
+		mBuffer.append(ch.in(), (U8*)"1", 1);
+		S32 capacity = mBuffer.capacity();
+		ensure("has capacity", capacity > 0);
+		U8* temp = new U8[capacity - 1];
+		mBuffer.append(ch.in(), temp, capacity - 1);
+		capacity = mBuffer.capacity();
+		ensure("has capacity when full", capacity > 0);
+		S32 used = mBuffer.countAfter(ch.in(), NULL);
+		ensure_equals("used equals capacity", used, capacity);
+
+		LLBufferArray::segment_iterator_t iter = mBuffer.beginSegment();
+		while(iter != mBuffer.endSegment())
+		{
+			mBuffer.eraseSegment(iter++);
+		}
+
+		used = mBuffer.countAfter(ch.in(), NULL);
+		ensure_equals("used is zero", used, 0);
+		S32 capacity2 = mBuffer.capacity();
+		ensure_equals("capacity the same after erase", capacity2, capacity);
+		mBuffer.append(ch.in(), temp, capacity - 1);
+		capacity2 = mBuffer.capacity();
+		ensure_equals("capacity the same after append", capacity2, capacity);
+
+		delete[] temp;
+	}
+
 #if 0
 	template<> template<>
 	void buffer_object::test<9>()
@@ -268,7 +394,7 @@ namespace tut
 	void bas_object::test<1>()
 	{
 		const char HELLO_WORLD[] = "hello world";
-		const S32 str_len = strlen(HELLO_WORLD);		/* Flawfinder: ignore */
+		const S32 str_len = strlen(HELLO_WORLD);	/* Flawfinder: ignore */
 		LLChannelDescriptors ch = mBuffer.nextChannel();
 		LLBufferStream str(ch, &mBuffer);
 		mBuffer.append(ch.in(), (U8*)HELLO_WORLD, str_len);