Fix segfault when reading from las into multiple buffers

In the presence of multiple buffers (such as the temporary buffers used by the Mosaic stage) caching the dimensions when reading las doesn't work: dimensions may be in different locations in different buffers. The previously used buffers may even be deallocated, in which case the cached dimensions will be stale pointers, leading to segfaults. The fix here simply removes dimension caching entirely. With any reasonable chunk size this shouldn't lead to performance degradation.
PDAL · Jun 29, 2013 · e3ab712 · e3ab712
1 parent 27ed088
commit e3ab712
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 27 deletions.
diff --git a/include/pdal/drivers/las/Reader.hpp b/include/pdal/drivers/las/Reader.hpp
@@ -144,10 +144,6 @@ class Base
     const pdal::drivers::las::Reader& m_reader;
     std::istream& m_istream;
 
-    PointDimensions* m_pointDimensions;
-    Schema const* m_schema;
-
-    void setPointDimensions(PointBuffer& buffer);
     inline pdal::drivers::las::Reader const& getReader()
     {
         return m_reader;

diff --git a/src/drivers/las/Reader.cpp b/src/drivers/las/Reader.cpp
@@ -681,8 +681,6 @@ namespace iterators
 Base::Base(pdal::drivers::las::Reader const& reader)
     : m_reader(reader)
     , m_istream(m_reader.getStreamFactory().allocate())
-    , m_pointDimensions(NULL)
-    , m_schema(0)
     , m_zipPoint(NULL)
     , m_unzipper(NULL)
 {
@@ -708,9 +706,6 @@ Base::~Base()
     m_unzipper.reset();
 #endif
 
-    if (m_pointDimensions)
-        delete m_pointDimensions;
-
     m_reader.getStreamFactory().deallocate(m_istream);
 }
 
@@ -755,16 +750,6 @@ void Base::read(PointBuffer&)
 
 }
 
-void Base::setPointDimensions(PointBuffer& buffer)
-{
-    // Cache dimension positions
-    Schema const& schema = buffer.getSchema();
-    if (m_pointDimensions)
-        delete m_pointDimensions;
-    m_pointDimensions = new PointDimensions(schema, m_reader.getName());
-
-}
-
 namespace sequential
 {
 
@@ -790,8 +775,6 @@ void Reader::readBeginImpl()
 
 void Reader::readBufferBeginImpl(PointBuffer& buffer)
 {
-    if (!m_pointDimensions)
-        setPointDimensions(buffer);
 }
 
 boost::uint64_t Reader::skipImpl(boost::uint64_t count)
@@ -823,21 +806,22 @@ bool Reader::atEndImpl() const
 
 boost::uint32_t Reader::readBufferImpl(PointBuffer& data)
 {
+    PointDimensions cachedDimensions(data.getSchema(), m_reader.getName());
 #ifdef PDAL_HAVE_LASZIP
     return m_reader.processBuffer(data,
                                   m_istream,
                                   getStage().getNumPoints()-this->getIndex(),
                                   m_unzipper.get(),
                                   m_zipPoint.get(),
-                                  m_pointDimensions,
+                                  &cachedDimensions,
                                   m_read_buffer);
 #else
     return m_reader.processBuffer(data,
                                   m_istream,
                                   getStage().getNumPoints()-this->getIndex(),
                                   NULL,
                                   NULL,
-                                  m_pointDimensions,
+                                  &cachedDimensions,
                                   m_read_buffer);
 
 #endif
@@ -868,8 +852,6 @@ void Reader::readBufferBeginImpl(PointBuffer& /* buffer*/)
 }
 void Reader::readBeginImpl()
 {
-    if (!m_pointDimensions)
-        setPointDimensions(getBuffer());
 }
 
 boost::uint64_t Reader::seekImpl(boost::uint64_t count)
@@ -898,21 +880,22 @@ boost::uint64_t Reader::seekImpl(boost::uint64_t count)
 
 boost::uint32_t Reader::readBufferImpl(PointBuffer& data)
 {
+    PointDimensions cachedDimensions(data.getSchema(), m_reader.getName());
 #ifdef PDAL_HAVE_LASZIP
     return m_reader.processBuffer(data,
                                   m_istream,
                                   getStage().getNumPoints()-this->getIndex(),
                                   m_unzipper.get(),
                                   m_zipPoint.get(),
-                                  m_pointDimensions,
+                                  &cachedDimensions,
                                   m_read_buffer);
 #else
     return m_reader.processBuffer(data,
                                   m_istream,
                                   getStage().getNumPoints()-this->getIndex(),
                                   NULL,
                                   NULL,
-                                  m_pointDimensions,
+                                  &cachedDimensions,
                                   m_read_buffer);
 
 #endif