[Wesnoth-cvs-commits] wesnoth/src network.cpp thread.cpp thread.hpp (June 03, 2005 - 02:06)

 

CVSROOT:	/cvsroot/wesnoth
Module name:	wesnoth
Branch: 	
Changes by:	David White <Sirp@xxxxxxxxxxxxxxxx>	05/06/02 23:58:02

Modified files:
	src            : network.cpp thread.cpp thread.hpp 

Log message:
	fixed potential crash in threading to connect to server

CVSWeb URLs:
http://savannah.gnu.org/cgi-bin/viewcvs/wesnoth/wesnoth/src/network.cpp.diff?tr1=1.62&tr2=1.63&r1=text&r2=text
http://savannah.gnu.org/cgi-bin/viewcvs/wesnoth/wesnoth/src/thread.cpp.diff?tr1=1.5&tr2=1.6&r1=text&r2=text
http://savannah.gnu.org/cgi-bin/viewcvs/wesnoth/wesnoth/src/thread.hpp.diff?tr1=1.3&tr2=1.4&r1=text&r2=text

Patches:
Index: wesnoth/src/network.cpp
diff -u wesnoth/src/network.cpp:1.62 wesnoth/src/network.cpp:1.63
--- wesnoth/src/network.cpp:1.62	Thu Jun  2 23:10:55 2005
+++ wesnoth/src/network.cpp	Thu Jun  2 23:58:01 2005
@@ -334,14 +334,14 @@
 
 connection connect(const std::string& host, int port, threading::waiter& waiter)
 {
-	connect_operation op(host,port);
-	const connect_operation::RESULT res = op.execute(waiter);
+	const threading::async_operation_holder<connect_operation> op(new connect_operation(host,port));
+	const connect_operation::RESULT res = op.operation().execute(waiter);
 	if(res == connect_operation::ABORTED) {
 		return 0;
 	}
 
-	op.check_error();
-	return op.result();
+	op.operation().check_error();
+	return op.operation().result();
 }
 
 connection accept_connection()
Index: wesnoth/src/thread.cpp
diff -u wesnoth/src/thread.cpp:1.5 wesnoth/src/thread.cpp:1.6
--- wesnoth/src/thread.cpp:1.5	Thu Jun  2 23:10:55 2005
+++ wesnoth/src/thread.cpp	Thu Jun  2 23:58:01 2005
@@ -11,7 +11,19 @@
 {
 	threading::async_operation* const op = reinterpret_cast<threading::async_operation*>(data);
 	op->run();
-	op->notify_finished(); //in case the operation didn't notify of finishing
+
+	bool should_delete = false;
+	{
+		const threading::lock l(op->get_mutex());
+		op->notify_finished(); //in case the operation didn't notify of finishing
+		if(op->is_aborted()) {
+			should_delete = true;
+		}
+	}
+
+	if(should_delete) {
+		delete op;
+	}
 
 	return 0;
 }
@@ -122,7 +134,6 @@
 	thread t(run_async_operation,this);
 
 	while(wait.process() == waiter::WAIT) {
-		std::cerr << "process...\n";
 		const condition::WAIT_TIMEOUT_RESULT res = finished_.wait_timeout(get_mutex(),20);
 		if(res == condition::WAIT_OK) {
 			return COMPLETED;
Index: wesnoth/src/thread.hpp
diff -u wesnoth/src/thread.hpp:1.3 wesnoth/src/thread.hpp:1.4
--- wesnoth/src/thread.hpp:1.3	Thu Jun  2 23:10:55 2005
+++ wesnoth/src/thread.hpp	Thu Jun  2 23:58:01 2005
@@ -168,6 +168,7 @@
 	SDL_cond* const cond_;
 };
 
+//class which defines an interface for waiting on an asynchronous operation
 class waiter {
 public:
 	enum ACTION { WAIT, ABORT };
@@ -176,6 +177,15 @@
 	virtual ACTION process() = 0;
 };
 
+//class which defines an asynchronous operation. Objects of this class are accessed from
+//both the worker thread and the calling thread, and so it has 'strange' allocation semantics.
+//It is allocated by the caller, and generally deleted by the caller. However, in some cases
+//the asynchronous operation is aborted, and the caller abandons it. The caller cannot still
+//delete the operation, since the worker thread might still access it, so in the case when the
+//operation is aborted, the worker thread will delete it.
+//
+//The caller should hold these objects using the async_operation_holder class below, which will
+//handle the delete semantics
 class async_operation
 {
 public:
@@ -197,15 +207,35 @@
 	//will be called in any case after the operation returns
 	void notify_finished();
 
-protected:
-
-	//must hold the mutex before calling this function
+	//must hold the mutex before calling this function from the worker thread
 	bool is_aborted() const { return aborted_; }
 
 private:
 	bool aborted_;
 	condition finished_;
 	mutex mutex_;
+};
+
+//T should be a type derived from async_operation
+template<typename T>
+class async_operation_holder
+{
+public:
+	explicit async_operation_holder(T* op) : op_(op)
+	{}
+
+	~async_operation_holder() {
+		//it's okay to call is_aborted() without the mutex here,
+		//because we are in the calling thread, not the worker thread
+		if(op_->is_aborted() == false) {
+			delete op_;
+		}
+	}
+
+	T& operation() const { return *op_; }
+
+private:
+	T* const op_;
 };
 
 }



You are on the gna.org mail server.

Generated by mhonarc, Tue Sep 20 16:50:21 2005