From: Mikko Rasa <tdb@tdb.fi>
Date: Sun, 19 Oct 2014 13:33:01 +0000 (+0300)
Subject: Synchronize access to the underlying object of Slice
X-Git-Url: http://git.tdb.fi/?p=libs%2Fcore.git;a=commitdiff_plain;h=169ece8b190040c4b61d68122c4ead1ba266cf79

Synchronize access to the underlying object of Slice

This was prompted by the interactions between GL::ResourceManager and
some upcoming changes to DataFile::PackSource.  If a new Slice object is
created and tries to connect to signal_flush_required at the exact moment
another Slice is seeking and causing the signal to be emitted, the emit
and connect calls step on each others' toes and corrupt the heap.
---

diff --git a/source/io/base.cpp b/source/io/base.cpp
index 052d464..da2b5a9 100644
--- a/source/io/base.cpp
+++ b/source/io/base.cpp
@@ -8,12 +8,14 @@ namespace IO {
 
 Base::Base():
 	mode(M_READ),
-	eof_flag(false)
+	eof_flag(false),
+	mutex(0)
 { }
 
 Base::~Base()
 {
 	signal_deleted.emit();
+	delete mutex;
 }
 
 void Base::check_access(Mode m) const
@@ -62,5 +64,19 @@ const Handle &Base::get_handle(Mode)
 	throw logic_error("Base::get_handle");
 }
 
+
+Base::Synchronize::Synchronize(Base &i):
+	io(i)
+{
+	if(!io.mutex)
+		io.mutex = new Mutex;
+	io.mutex->lock();
+}
+
+Base::Synchronize::~Synchronize()
+{
+	io.mutex->unlock();
+}
+
 } // namespace IO
 } // namespace Msp
diff --git a/source/io/base.h b/source/io/base.h
index 394a2c2..c58be90 100644
--- a/source/io/base.h
+++ b/source/io/base.h
@@ -2,6 +2,7 @@
 #define MSP_IO_BASE_H_
 
 #include <sigc++/sigc++.h>
+#include <msp/core/mutex.h>
 #include "handle.h"
 #include "mode.h"
 #include "poll.h"
@@ -18,6 +19,18 @@ leaving stale pointers in an EventDispatcher.
 class Base
 {
 public:
+	/** RAII synchronization primitive.  Prevents concurrent access to the
+	target object during the lifetime of the Synchronize object. */
+	class Synchronize
+	{
+	private:
+		Base &io;
+
+	public:
+		Synchronize(Base &);
+		~Synchronize();
+	};
+
 	/** Emitted when there is no more data to be read. */
 	sigc::signal<void> signal_end_of_file;
 
@@ -32,6 +45,7 @@ public:
 protected:
 	Mode mode;
 	bool eof_flag;
+	Mutex *mutex;
 
 	Base();
 private:
diff --git a/source/io/slice.cpp b/source/io/slice.cpp
index 904f748..59a7cde 100644
--- a/source/io/slice.cpp
+++ b/source/io/slice.cpp
@@ -16,6 +16,7 @@ Slice::Slice(Seekable &b, SeekOffset s, SeekOffset l):
 	if(s<0 || l<0)
 		throw invalid_argument("Slice");
 
+	Base::Synchronize sync(below);
 	mode = below.get_mode()&M_RDWR;
 	below.signal_flush_required.connect(sigc::mem_fun(this, &Slice::flush));
 }
@@ -30,7 +31,12 @@ unsigned Slice::prepare_op(unsigned size, Mode m)
 	check_access(m);
 
 	if(sync_position)
-		seek(position, S_BEG);
+	{
+		SeekOffset pos = below.seek(start_offset+position, S_BEG)-start_offset;
+		if(pos!=position)
+			throw runtime_error("failed to restore position");
+		sync_position = false;
+	}
 
 	SeekOffset remaining = start_offset+length-position;
 	if(size>remaining)
@@ -43,6 +49,7 @@ unsigned Slice::prepare_op(unsigned size, Mode m)
 
 unsigned Slice::do_write(const char *data, unsigned size)
 {
+	Base::Synchronize sync(below);
 	size = prepare_op(size, M_WRITE);
 	if(!size)
 		return 0;
@@ -54,6 +61,7 @@ unsigned Slice::do_write(const char *data, unsigned size)
 
 unsigned Slice::do_read(char *data, unsigned size)
 {
+	Base::Synchronize sync(below);
 	size = prepare_op(size, M_READ);
 	if(!size)
 		return 0;
@@ -67,6 +75,7 @@ unsigned Slice::do_read(char *data, unsigned size)
 
 unsigned Slice::put(char c)
 {
+	Base::Synchronize sync(below);
 	if(!prepare_op(1, M_WRITE))
 		return 0;
 
@@ -77,6 +86,7 @@ unsigned Slice::put(char c)
 
 int Slice::get()
 {
+	Base::Synchronize sync(below);
 	if(!prepare_op(1, M_READ))
 		return 0;
 
@@ -90,6 +100,7 @@ int Slice::get()
 
 SeekOffset Slice::seek(SeekOffset off, SeekType type)
 {
+	Base::Synchronize sync(below);
 	off += start_offset;
 	if(type==S_CUR)
 		off += position;