Make the server suspend the core around calls unless disabled.

I expect the majority of methods will have to suspend the core
first thing anyway, so this will reduce typing and bugs.

Also get rid of the "me->" ugliness by splitting threadFn.

library/RemoteServer.cpp

@@ -198,6 +198,14 @@ void ServerConnection::connection_ostream::flush_proxy()
 void ServerConnection::threadFn(void *arg)
 {
     ServerConnection *me = (ServerConnection*)arg;
+
+    me->threadFn();
+
+    delete me;
+}
+
+void ServerConnection::threadFn()
+{
     color_ostream_proxy out(Core::getInstance().getConsole());
 
     /* Handshake */
@@ -205,10 +213,9 @@ void ServerConnection::threadFn(void *arg)
     {
         RPCHandshakeHeader header;
 
-        if (!readFullBuffer(me->socket, &header, sizeof(header)))
+        if (!readFullBuffer(socket, &header, sizeof(header)))
         {
             out << "In RPC server: could not read handshake header." << endl;
-            delete me;
             return;
         }
 
@@ -216,17 +223,15 @@ void ServerConnection::threadFn(void *arg)
             header.version != 1)
         {
             out << "In RPC server: invalid handshake header." << endl;
-            delete me;
             return;
         }
 
         memcpy(header.magic, RPCHandshakeHeader::RESPONSE_MAGIC, sizeof(header.magic));
         header.version = 1;
 
-        if (me->socket->Send((uint8*)&header, sizeof(header)) != sizeof(header))
+        if (socket->Send((uint8*)&header, sizeof(header)) != sizeof(header))
         {
             out << "In RPC server: could not send handshake response." << endl;
-            delete me;
             return;
         }
     }
@@ -235,11 +240,11 @@ void ServerConnection::threadFn(void *arg)
 
     std::cerr << "Client connection established." << endl;
 
-    while (!me->in_error) {
+    while (!in_error) {
         // Read the message
         RPCMessageHeader header;
 
-        if (!readFullBuffer(me->socket, &header, sizeof(header)))
+        if (!readFullBuffer(socket, &header, sizeof(header)))
         {
             out.printerr("In RPC server: I/O error in receive header.\n");
             break;
@@ -256,7 +261,7 @@ void ServerConnection::threadFn(void *arg)
 
         std::auto_ptr<uint8_t> buf(new uint8_t[header.size]);
 
-        if (!readFullBuffer(me->socket, buf.get(), header.size))
+        if (!readFullBuffer(socket, buf.get(), header.size))
         {
             out.printerr("In RPC server: I/O error in receive %d bytes of data.\n", header.size);
             break;
@@ -267,31 +272,40 @@ void ServerConnection::threadFn(void *arg)
         // Find and call the function
         int in_size = header.size;
 
-        ServerFunctionBase *fn = vector_get(me->functions, header.id);
+        ServerFunctionBase *fn = vector_get(functions, header.id);
         MessageLite *reply = NULL;
         command_result res = CR_FAILURE;
 
         if (!fn)
         {
-            me->stream.printerr("RPC call of invalid id %d\n", header.id);
+            stream.printerr("RPC call of invalid id %d\n", header.id);
         }
         else
         {
             if (!fn->in()->ParseFromArray(buf.get(), header.size))
             {
-                me->stream.printerr("In call to %s: could not decode input args.\n", fn->name);
+                stream.printerr("In call to %s: could not decode input args.\n", fn->name);
             }
             else
             {
                 buf.reset();
 
                 reply = fn->out();
-                res = fn->execute(me->stream);
+
+                if (fn->flags & SF_DONT_SUSPEND)
+                {
+                    res = fn->execute(stream);
+                }
+                else
+                {
+                    CoreSuspender suspend;
+                    res = fn->execute(stream);
+                }
             }
         }
 
         // Flush all text output
-        if (me->in_error)
+        if (in_error)
             break;
 
         //out.print("Answer %d:%d\n", res, reply);
@@ -301,16 +315,16 @@ void ServerConnection::threadFn(void *arg)
 
         if (out_size > RPCMessageHeader::MAX_MESSAGE_SIZE)
         {
-            me->stream.printerr("In call to %s: reply too large: %d.\n",
+            stream.printerr("In call to %s: reply too large: %d.\n",
                                 (fn ? fn->name : "UNKNOWN"), out_size);
             res = CR_LINK_FAILURE;
         }
 
-        me->stream.flush();
+        stream.flush();
 
         if (res == CR_OK && reply)
         {
-            if (!sendRemoteMessage(me->socket, RPC_REPLY_RESULT, reply, true))
+            if (!sendRemoteMessage(socket, RPC_REPLY_RESULT, reply, true))
             {
                 out.printerr("In RPC server: I/O error in send result.\n");
                 break;
@@ -321,7 +335,7 @@ void ServerConnection::threadFn(void *arg)
             header.id = RPC_REPLY_FAIL;
             header.size = res;
 
-            if (me->socket->Send((uint8_t*)&header, sizeof(header)) != sizeof(header))
+            if (socket->Send((uint8_t*)&header, sizeof(header)) != sizeof(header))
             {
                 out.printerr("In RPC server: I/O error in send failure code.\n");
                 break;
@@ -337,8 +351,6 @@ void ServerConnection::threadFn(void *arg)
     }
 
     std::cerr << "Shutting down client connection." << endl;
-
-    delete me;
 }
 
 ServerMain::ServerMain()

library/RemoteTools.cpp

@@ -217,6 +217,7 @@ void DFHack::describeName(NameInfo *info, df::language_name *name)
     std::string lname = Translation::TranslateName(name, false, true);
     if (!lname.empty())
         info->set_last_name(lname);
+
     lname = Translation::TranslateName(name, true, true);
     if (!lname.empty())
         info->set_english_name(lname);
@@ -276,13 +277,17 @@ static command_result ListEnums(color_ostream &stream,
 {
 #define ENUM(name) describe_enum<df::name>(out->mutable_##name());
 #define BITFIELD(name) describe_bitfield<df::name>(out->mutable_##name());
+
     ENUM(material_flags);
     ENUM(inorganic_flags);
+
     BITFIELD(unit_flags1);
     BITFIELD(unit_flags2);
     BITFIELD(unit_flags3);
+
     ENUM(unit_labor);
     ENUM(job_skill);
+
 #undef ENUM
 #undef BITFIELD
 }
@@ -297,8 +302,6 @@ static void listMaterial(ListMaterialsOut *out, int type, int index, const Basic
 static command_result ListMaterials(color_ostream &stream,
                                     const ListMaterialsIn *in, ListMaterialsOut *out)
 {
-    CoreSuspender suspend;
-
     auto mask = in->has_mask() ? &in->mask() : NULL;
 
     for (int i = 0; i < in->id_list_size(); i++)
@@ -350,8 +353,6 @@ static command_result ListMaterials(color_ostream &stream,
 static command_result ListUnits(color_ostream &stream,
                                 const ListUnitsIn *in, ListUnitsOut *out)
 {
-    CoreSuspender suspend;
-
     auto mask = in->has_mask() ? &in->mask() : NULL;
 
     if (in->id_list_size() > 0)
@@ -415,14 +416,15 @@ CoreService::CoreService() {
     suspend_depth = 0;
 
     // These 2 methods must be first, so that they get id 0 and 1
-    addMethod("BindMethod", &CoreService::BindMethod);
+    addMethod("BindMethod", &CoreService::BindMethod, SF_DONT_SUSPEND);
-    addMethod("RunCommand", &CoreService::RunCommand);
+    addMethod("RunCommand", &CoreService::RunCommand, SF_DONT_SUSPEND);
 
     // Add others here:
-    addMethod("CoreSuspend", &CoreService::CoreSuspend);
+    addMethod("CoreSuspend", &CoreService::CoreSuspend, SF_DONT_SUSPEND);
-    addMethod("CoreResume", &CoreService::CoreResume);
+    addMethod("CoreResume", &CoreService::CoreResume, SF_DONT_SUSPEND);
 
-    addFunction("ListEnums", ListEnums, SF_CALLED_ONCE);
+    // Functions:
+    addFunction("ListEnums", ListEnums, SF_CALLED_ONCE | SF_DONT_SUSPEND);
     addFunction("ListMaterials", ListMaterials, SF_CALLED_ONCE);
     addFunction("ListUnits", ListUnits);
     addFunction("ListSquads", ListSquads);

library/include/RemoteServer.h

@@ -43,7 +43,10 @@ namespace  DFHack
     enum ServerFunctionFlags {
         // The function is expected to be called only once per client,
         // so always delete all cached buffers after processing.
-        SF_CALLED_ONCE = 1
+        SF_CALLED_ONCE = 1,
+        // Don't automatically suspend the core around the call.
+        // The function is supposed to manage locking itself.
+        SF_DONT_SUSPEND = 2
     };
 
     class DFHACK_EXPORT ServerFunctionBase : public RPCFunctionBase {
@@ -225,6 +228,7 @@ namespace  DFHack
 
         tthread::thread *thread;
         static void threadFn(void *);
+        void threadFn();
 
     public:
         ServerConnection(CActiveSocket *socket);

library/proto/Basic.proto

@@ -5,6 +5,8 @@ option optimize_for = LITE_RUNTIME;
 message EnumItemName {
     required int32 value = 1;
     optional string name = 2;
+
+    // For bitfield members
     optional int32 bit_size = 3 [default = 1];
 };
 
@@ -16,10 +18,15 @@ message BasicMaterialId {
 message BasicMaterialInfo {
     required int32 type = 1;
     required sint32 index = 2;
+
+    // The raw token
     required string token = 3;
 
-    repeated int32 flags = 4; // of material_flags
+    // IF mask.flags:
+    // List of material_flags indices
+    repeated int32 flags = 4;
 
+    // Material type/index expanded:
     optional int32 subtype = 5 [default = -1];
     optional int32 creature_id = 6 [default = -1];
     optional int32 plant_id = 7 [default = -1];
@@ -27,10 +34,13 @@ message BasicMaterialInfo {
 
     optional string name_prefix = 9 [default = ""];
 
+    // IF mask.states: in listed order;
+    // ELSE: one state matching mask.temperature
     repeated fixed32 state_color = 10;
     repeated string state_name = 11;
     repeated string state_adj = 12;
 
+    // IF mask.reaction:
     message Product {
         required string id = 1;
         required int32 type = 2;
@@ -39,6 +49,7 @@ message BasicMaterialInfo {
     repeated string reaction_class = 13;
     repeated Product reaction_product = 14;
 
+    // IF mask.flags:
     repeated int32 inorganic_flags = 15;
 };
 
@@ -90,8 +101,10 @@ message BasicUnitInfo {
     optional int32 civ_id = 9 [default = -1];
     optional int32 histfig_id = 10 [default = -1];
 
+    // IF mask.labors:
     repeated int32 labors = 11;
 
+    // IF mask.skills:
     repeated SkillInfo skills = 12;
 
     required int32 pos_x = 13;
@@ -108,7 +121,10 @@ message BasicSquadInfo {
     required int32 squad_id = 1;
 
     optional NameInfo name = 2;
+
+    // A special field completely overriding the name:
     optional string alias = 3;
 
+    // Member histfig ids:
     repeated sint32 members = 4;
 };

library/proto/BasicApi.proto

@@ -18,7 +18,11 @@ message ListEnumsOut {
 
 message ListMaterialsIn {
     optional BasicMaterialInfoMask mask = 1;
+
+    // Specific materials:
     repeated BasicMaterialId id_list = 2;
+
+    // Complete list by type:
     optional bool builtin = 3;
     optional bool inorganic = 4;
     optional bool creatures = 5;
@@ -30,8 +34,11 @@ message ListMaterialsOut {
 
 message ListUnitsIn {
     optional BasicUnitInfoMask mask = 1;
+
+    // Specific units:
     repeated int32 id_list = 2;
 
+    // All units matching:
     optional int32 race = 3;
     optional int32 civ_id = 4;
 };

plugins/rename.cpp

@@ -124,8 +124,6 @@ void setUnitNickname(df::unit *unit, const std::string &nick)
 
 static command_result RenameSquad(color_ostream &stream, const RenameSquadIn *in)
 {
-    CoreSuspender suspend;
-
     df::squad *squad = df::squad::find(in->squad_id());
     if (!squad)
         return CR_NOT_FOUND;
@@ -140,8 +138,6 @@ static command_result RenameSquad(color_ostream &stream, const RenameSquadIn *in
 
 static command_result RenameUnit(color_ostream &stream, const RenameUnitIn *in)
 {
-    CoreSuspender suspend;
-
     df::unit *unit = df::unit::find(in->unit_id());
     if (!unit)
         return CR_NOT_FOUND;