Memory leak in the pDataBlock copy constructor.

[csgro]

We just committed this change

diff --git a/daq/pDataBlock.cpp b/daq/pDataBlock.cpp
index 54139b4..b5291a7 100644
--- a/daq/pDataBlock.cpp
+++ b/daq/pDataBlock.cpp
@@ -70,18 +70,22 @@ pDataBlock::pDataBlock(unsigned char *buffer, unsigned int b
 }

-/*!
+/*!The amount of usable data in the buffer read from the USB port depends on
+  the readout mode and is effectively:
+  - 2 * NWORDS = 2 * 13200 * 8 = 211200 bytes in full frame mode (exactly one
+  event).
+  - < 10000 bytes in windowed mode with the small buffer (with the last event
+  being truncated).
+  - < 2 * 2**18 = 2* 262144 = 2 * SRAM_DIM = 524288 bytes (with the last event
+  being truncated).
 */
 pDataBlock::pDataBlock(const pDataBlock &cSourceDataBlock) :
-                         m_isWindowed (cSourceDataBlock.m_isWindowed)
+  m_isWindowed (cSourceDataBlock.m_isWindowed)
 {
   m_size = cSourceDataBlock.m_size;
-  if (cSourceDataBlock.m_rawBuffer)
-  {
-    /* Awful. Isn't there a better way to copy the buffer? */    
-    char temp_buffer [2*NWORDS];
-    memcpy(temp_buffer, cSourceDataBlock.getCharDataBlock(), m_size);
-    m_rawBuffer = reinterpret_cast<unsigned char*> (temp_buffer);
+  if (cSourceDataBlock.m_rawBuffer) {
+    m_rawBuffer = new unsigned char[m_size];
+    memcpy(m_rawBuffer, cSourceDataBlock.m_rawBuffer, m_size);
   }
   m_errorSummary = cSourceDataBlock.m_errorSummary;
   m_offsetVec = cSourceDataBlock.m_offsetVec;
(END)

The underlying m_rawBuffer is created via new and the pDataBlock destructor is a do-nothing one, so this is effectively a memory leak. In the main data acquisition class the buffer is created statically, which is why we're not deleting it in the destructor. The right thing to do would probably be to use new in pDataCollector and implement a real destructor.

[csgro]

A related leak affects also the monitor, since the function pEventReader::readPendingDatagram creates the buffer dynamically and never delete it. For now the problem has been fixed by destroying manually the buffer after the use. I suggest leaving the task to the pDataBlock destructor.

[albertomanfreda]

We have a fix for this leak, now. In the new scheme each pDataBlock owns its underlying data buffer, and is responsible for releasing the memory when it goes out of scope. The relevant code is in the pDataBlock destructor:

+/*!Destructor. Delete the underlying data buffer.
 +*/
 +pDataBlock::~pDataBlock()
 +{
 +  if (m_rawBuffer) delete[] m_rawBuffer;
 +}
 +
 +

A consequence of this change is that now, in the main loop of pDataCollector::run(), we need to instantiate a new buffer at each iteration, as opposed to overwrite always the same buffer instantiated at the beginning. The change has been propagated in the pEventReader::readDatagram() function of the monitor application.