Merge pull request #60 from mfbonfigli/bugfix/59

Fix race condition in combined LAS reader

Author: mfbonfigli

README.md

@@ -59,6 +59,9 @@ You can preview a couple of tilesets generated from this tool at this [website](
 
 
 ## Changelog
+##### Version 2.0.1
+* Resolved a bug that prevented the correct functioning of the join flag when processing multiple LAS files from a folder.
+
 ##### Version 2.0.0
 * Most of the code has been rewritten from the ground up, achieving much faster tiling with lower memory usage
 * Uses Proj v9.5.0: all projections supported by the Proj library are automatically supported by gocesiumtiler.

cmd/main.go

@@ -23,7 +23,7 @@ var tilerProvider func() (tiler.Tiler, error) = func() (tiler.Tiler, error) {
 	return tiler.NewGoCesiumTiler()
 }
 
-var cmdVersion = "2.0.0"
+var cmdVersion = "2.0.1"
 
 // GitCommit is injected dynamically at build time via `go build -ldflags "-X main.GitCommit=XYZ"`
 var GitCommit string = "(na)"

internal/las/golas/reader.go

@@ -219,6 +219,7 @@ func (g *Las) Next() (Point, error) {
 	// a Seek operation right before returning the struct
 	g.Lock()
 	if g.current >= g.NumberOfPoints() {
+		g.Unlock()
 		return p, io.EOF
 	}
 	data := make([]byte, g.Header.PointDataRecordLength)

internal/las/reader.go

@@ -2,7 +2,9 @@ package las
 
 import (
 	"fmt"
+	"io"
 	"os"
+	"sync/atomic"
 
 	"github.com/mfbonfigli/gocesiumtiler/v2/internal/geom"
 	"github.com/mfbonfigli/gocesiumtiler/v2/internal/las/golas"
@@ -24,8 +26,7 @@ type LasReader interface {
 // CombinedFileLasReader enables reading a a list of LAS files as if they were a single one
 // the files MUST have the same properties (SRID, etc)
 type CombinedFileLasReader struct {
-	currentReader int
-	currentCount  int
+	currentReader atomic.Int32
 	readers       []LasReader
 	numPts        int
 	crs           string
@@ -64,20 +65,19 @@ func (m *CombinedFileLasReader) GetCRS() string {
 }
 
 func (m *CombinedFileLasReader) GetNext() (geom.Point64, error) {
-	if m.currentReader >= len(m.readers) {
-		return geom.Point64{}, fmt.Errorf("no points to read")
-	}
-	r := m.readers[m.currentReader]
-	if m.currentCount == r.NumberOfPoints() {
-		m.currentReader++
-		m.currentCount = 0
-		if m.currentReader >= len(m.readers) {
-			return geom.Point64{}, fmt.Errorf("no points to read")
+	for {
+		currReader := int(m.currentReader.Load())
+		if currReader >= len(m.readers) {
+			return geom.Point64{}, io.EOF
+		}
+		pt, err := m.readers[currReader].GetNext()
+		if err != nil {
+			// try to move on to the next reader
+			m.currentReader.CompareAndSwap(int32(currReader), int32(currReader)+1)
+			continue
 		}
-		r = m.readers[m.currentReader]
+		return pt, nil
 	}
-	m.currentCount++
-	return r.GetNext()
 }
 
 func (m *CombinedFileLasReader) Close() {

internal/las/reader_test.go

@@ -3,6 +3,7 @@ package las
 import (
 	"fmt"
 	"os"
+	"sync"
 	"testing"
 )
 
@@ -42,3 +43,54 @@ func TestCombinedReader(t *testing.T) {
 		t.Errorf("expected error, got none")
 	}
 }
+
+func TestCombinedReaderConcurrency(t *testing.T) {
+	entries, err := os.ReadDir("./testdata")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	files := []string{}
+	for _, e := range entries {
+		filename := e.Name()
+		files = append(files, fmt.Sprintf("./testdata/%s", filename))
+	}
+
+	r, err := NewCombinedFileLasReader(files, "EPSG:32633", false)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if actual := r.NumberOfPoints(); actual != 10*len(files) {
+		t.Errorf("expected %d points got %d", 10*len(files), actual)
+	}
+
+	if actual := r.GetCRS(); actual != "EPSG:32633" {
+		t.Errorf("expected epsg %d got epsg %s", 32633, actual)
+	}
+
+	e := make(chan error, 10)
+	readFun := func(wg *sync.WaitGroup) {
+		defer wg.Done()
+		read := 0
+		for i := 0; i < r.NumberOfPoints()/5; i++ {
+			_, err := r.GetNext()
+			if err != nil {
+				e <- err
+				t.Errorf("unexpected error %v", err)
+				continue
+			}
+			read++
+		}
+		fmt.Println(read)
+	}
+	wg := &sync.WaitGroup{}
+	for i := 0; i < 5; i++ {
+		wg.Add(1)
+		go readFun(wg)
+	}
+	wg.Wait()
+	if len(e) > 0 {
+		t.Errorf("errors detected in the error channel but none expected")
+	}
+}