Fixed crash in online user search (triggered when pressing ESC while the

search is being executed).

src/online/current_user.cpp

@@ -366,7 +366,7 @@ namespace Online
      *  search term.
      *  \param search_string the string to search for.
      */
-    const XMLRequest*
+    XMLRequest*
         CurrentUser::requestUserSearch(const core::stringw &search_string) const
     {
         assert(m_state == US_SIGNED_IN);

src/online/current_user.hpp

@@ -196,7 +196,7 @@ namespace Online
                                                                     const irr::core::stringw &new_password,
                                                                     const irr::core::stringw &new_password_ver) const;
 
-            const XMLRequest *              requestUserSearch(const irr::core::stringw & search_string) const;
+            XMLRequest *                    requestUserSearch(const irr::core::stringw & search_string) const;
 
             void                            onSTKQuit() const;
             void                            onAchieving(uint32_t achievement_id) const;

src/online/request.hpp

@@ -66,15 +66,17 @@ namespace Online
         LEAK_CHECK()
 
         /** Type of the request. Has 0 as default value. */
-        const int              m_type;
+        const int m_type;
+
         /** True if the memory for this Request should be managed by
         *  http connector (i.e. this object is freed once the request
         *  is handled). Otherwise the memory is not freed, so it must
         *  be freed by the calling function. */
-        const bool             m_manage_memory;
+        bool m_manage_memory;
+
         /** The priority of this request. The higher the value the more
         important this request is. */
-        const int              m_priority;
+        const int m_priority;
 
         /** The different state of the requst:
          *  - S_PREPARING:\n The request is created and can be configured, it
@@ -140,6 +142,11 @@ namespace Online
         *  by network_http (i.e. freed once the request is handled). */
         bool manageMemory() const   { return m_manage_memory; }
         // --------------------------------------------------------------------
+        /** Sets the memory management flag of this request. This function
+         *  must only be called by the main thread, since it is only tested by
+         *  the main thread. */
+        void setManageMemory(bool m) { m_manage_memory = m;  }
+        // --------------------------------------------------------------------
         /** Returns the priority of this request. */
         int getPriority() const   { return m_priority; }
         // --------------------------------------------------------------------

src/online/request_manager.cpp

@@ -263,7 +263,7 @@ namespace Online
             else
                 request->setDone();
         }
-    }
+    }   // handleResultQueue
 
     // ------------------------------------------------------------------------
     /** Should be called every frame and takes care of processing the result

src/states_screens/online_user_search.cpp

@@ -88,8 +88,46 @@ void OnlineUserSearch::init()
  */
 void OnlineUserSearch::tearDown()
 {
-    delete m_search_request;
-    m_search_request = NULL;
+    // The search request can be in one of three states:
+    // 1. It does not exist, nothing more to do.
+    // 2. It has been executed by the request manager, had its callback done
+    //    and waits for this widget to collect the results. In this case, the
+    //    requests state is 'isDone', and the memory of this object need to be
+    //    freed here.
+    // 3. It is being executed by the request manager thread. In this case the
+    //    request can not be freed (since the request manager might still
+    //    write to it). In this case we set the flag that the request manager
+    //    should manage the memory for the request, i.e. the request will be
+    //    deleted once it is in the request manager's ready queue.
+    // Note that there is no race condition here: setting a request to be
+    // 'done', and checking if its memory need to be freed is done by the
+    // main thread (i.e. the same thread that executes this function ). So it
+    // is not possible that the thread stage changes to be 'isDone' while
+    // this function executes, or that the request is checked if it should
+    // be freed.
+
+    if (m_search_request)
+    {
+        // Check if the request is ready (but its result have not been
+        // received here ... otherwise it would have been deleted).
+        if (m_search_request->isDone())
+        {
+            delete m_search_request;
+        }
+        else
+        {
+            // This request is currently being handled by the request manager.
+            // We can set the memory management flag, since the separate
+            // request manager thread does not read or write this flag.
+            // The actual deletion of the request is done by the main
+            // thread, so there is no risk that the request is deleted
+            // between the next two calls!!
+            m_search_request->setManageMemory(true);
+
+            // Set cancel flag to speed up cancellation of this request
+            m_search_request->cancel();
+        }
+    }   // if m_search_request
 }   // tearDown
 
 

src/states_screens/online_user_search.hpp

@@ -60,7 +60,7 @@ private:
     Online::Profile::IDList                     m_users;
 
     /** The online request to search for users. */
-    const Online::XMLRequest *                  m_search_request;
+    Online::XMLRequest *m_search_request;
 
     void parseResult(const XMLNode * input);
     void showList();