[Wesnoth-cvs-commits] wesnoth/src network.cpp thread.cpp thread.hpp

[David White]

CVSROOT:        /cvsroot/wesnoth
Module name:    wesnoth
Branch:         
Changes by:     David White <address@hidden>    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_;
 };
 
 }