Remove feature: reset PID on killall()

reseting PID was a very bad idea since we don't check on creation
whether a PID is already given. This was resulting in several
actors having the same PID.

This, with the previous commit, (fix #254), that was actually a team of
2 different bugs working together.

diff --git a/ChangeLog b/ChangeLog
index 827ae25..ba73427 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,8 @@ SimGrid (3.19) NOT RELEASED YET (target: March 20 2018, 16:15:27 UTC)
  MSG
  - Fix MSG_task_get_remaining_work_ratio() to return 1.0 for tasks that have
    not started.
+ - Remove parameter of MSG_process_killall().
+   Resetting the PID was bogus anyway (several actors could have the same PID).
 
  Documentation
  - Use a graphical TOC to make it easier to find the documentation you need
@@ -39,6 +41,9 @@ SimGrid (3.19) NOT RELEASED YET (target: March 20 2018, 16:15:27 UTC)
  TRACING
  - Remove unused run-time parameter "tracing/onelink-only".
 
+ Fixed bugs:
+ - #254: Something seems wrong with s4u::Actor::kill(aid_t)
+
 SimGrid (3.18) Released December 24 2017
 
  The "Ho Ho Ho! SimGrid 4 beta is coming to town" release.

diff --git a/examples/s4u/actor-kill-pid/s4u-actor-kill-pid.tesh b/examples/s4u/actor-kill-pid/s4u-actor-kill-pid.tesh
index e6171fc..79104cf 100644 (file)
--- a/examples/s4u/actor-kill-pid/s4u-actor-kill-pid.tesh
+++ b/examples/s4u/actor-kill-pid/s4u-actor-kill-pid.tesh
@@ -8,8 +8,8 @@ $ $SG_TEST_EXENV ${bindir:=.}/s4u-actor-kill-pid ${platfdir}/small_platform.xml
 >[ 10.000000] (killer@Tremblay) Resume the victim A
 >[ 10.000000] (victim A@Fafard) OK, OK. Let's work
 >[ 12.000000] (killer@Tremblay) Kill the victim A (pid=2)
+>[ 12.000000] (victim A@Fafard) I have been killed!
 >[ 12.000000] (killer@Tremblay) Kill victimB (pid=3), even if it's already dead
 >[ 13.000000] (killer@Tremblay) Killing everybody but myself
->[ 13.000000] (victim A@Fafard) I have been killed!
 >[ 13.000000] (killer@Tremblay) OK, goodbye now. I commit a suicide (pid=1).
 >[ 13.000000] (maestro@) Simulation time 13

diff --git a/include/simgrid/msg.h b/include/simgrid/msg.h
index b885231..420a9b2 100644 (file)
--- a/include/simgrid/msg.h
+++ b/include/simgrid/msg.h
@@ -221,7 +221,7 @@ XBT_PUBLIC(msg_process_t) MSG_process_attach(const char* name, void* data, msg_h
 XBT_PUBLIC(void) MSG_process_detach();
 
 XBT_PUBLIC(void) MSG_process_kill(msg_process_t process);
-XBT_PUBLIC(int) MSG_process_killall(int reset_PIDs);
+XBT_PUBLIC(int) MSG_process_killall();
 XBT_PUBLIC(msg_error_t) MSG_process_migrate(msg_process_t process, msg_host_t host);
 XBT_PUBLIC(void) MSG_process_yield();
 

diff --git a/include/simgrid/simix.h b/include/simgrid/simix.h
index 56c2f33..3d145b9 100644 (file)
--- a/include/simgrid/simix.h
+++ b/include/simgrid/simix.h
@@ -199,7 +199,7 @@ simcall_process_create(const char* name, xbt_main_func_t code, void* data, sg_ho
                        std::map<std::string, std::string>* properties);
 #endif
 
-XBT_PUBLIC(void) simcall_process_killall(int reset_pid);
+XBT_PUBLIC(void) simcall_process_killall();
 XBT_PUBLIC(void) SIMIX_process_throw(smx_actor_t process, xbt_errcat_t cat, int value, const char *msg);
 
 

diff --git a/src/msg/msg_global.cpp b/src/msg/msg_global.cpp
index aad5620..9845fdf 100644 (file)
--- a/src/msg/msg_global.cpp
+++ b/src/msg/msg_global.cpp
@@ -93,13 +93,10 @@ void MSG_config(const char *key, const char *value){
 /** \ingroup msg_simulation
  * \brief Kill all running process
 
- * \param reset_PIDs should we reset the PID numbers. A negative
- *   number means no reset and a positive number will be used to set the PID
- *   of the next newly created process.
  */
-int MSG_process_killall(int reset_PIDs)
+int MSG_process_killall()
 {
-  simcall_process_killall(reset_PIDs);
+  simcall_process_killall();
 
   return 0;
 }

diff --git a/src/s4u/s4u_actor.cpp b/src/s4u/s4u_actor.cpp
index 39c8ab6..10be224 100644 (file)
--- a/src/s4u/s4u_actor.cpp
+++ b/src/s4u/s4u_actor.cpp
@@ -203,12 +203,7 @@ ActorPtr Actor::byPid(aid_t pid)
 
 void Actor::killAll()
 {
-  simcall_process_killall(1);
-}
-
-void Actor::killAll(int resetPid)
-{
-  simcall_process_killall(resetPid);
+  simcall_process_killall();
 }
 
 std::map<std::string, std::string>* Actor::getProperties()

diff --git a/src/simix/ActorImpl.cpp b/src/simix/ActorImpl.cpp
index 56688a4..b93d874 100644 (file)
--- a/src/simix/ActorImpl.cpp
+++ b/src/simix/ActorImpl.cpp
@@ -575,21 +575,19 @@ void SIMIX_process_throw(smx_actor_t process, xbt_errcat_t cat, int value, const
 
 }
 
-void simcall_HANDLER_process_killall(smx_simcall_t simcall, int reset_pid) {
-  SIMIX_process_killall(simcall->issuer, reset_pid);
+void simcall_HANDLER_process_killall(smx_simcall_t simcall)
+{
+  SIMIX_process_killall(simcall->issuer);
 }
 /**
  * \brief Kills all running processes.
  * \param issuer this one will not be killed
  */
-void SIMIX_process_killall(smx_actor_t issuer, int reset_pid)
+void SIMIX_process_killall(smx_actor_t issuer)
 {
   for (auto const& kv : simix_global->process_list)
     if (kv.second != issuer)
       SIMIX_process_kill(kv.second, issuer);
-
-  if (reset_pid > 0)
-    simix_process_maxpid = reset_pid;
 }
 
 void SIMIX_process_change_host(smx_actor_t actor, sg_host_t dest)

diff --git a/src/simix/ActorImpl.hpp b/src/simix/ActorImpl.hpp
index fab1cea..ad51e8b 100644 (file)
--- a/src/simix/ActorImpl.hpp
+++ b/src/simix/ActorImpl.hpp
@@ -140,7 +140,7 @@ XBT_PRIVATE smx_actor_t SIMIX_process_create(const char* name, std::function<voi
 
 XBT_PRIVATE void SIMIX_process_runall();
 XBT_PRIVATE void SIMIX_process_kill(smx_actor_t process, smx_actor_t issuer);
-XBT_PRIVATE void SIMIX_process_killall(smx_actor_t issuer, int reset_pid);
+XBT_PRIVATE void SIMIX_process_killall(smx_actor_t issuer);
 XBT_PRIVATE void SIMIX_process_cleanup(smx_actor_t arg);
 XBT_PRIVATE void SIMIX_process_empty_trash();
 XBT_PRIVATE void SIMIX_process_yield(smx_actor_t self);

diff --git a/src/simix/libsmx.cpp b/src/simix/libsmx.cpp
index 61d57bc..15f0a2e 100644 (file)
--- a/src/simix/libsmx.cpp
+++ b/src/simix/libsmx.cpp
@@ -180,9 +180,9 @@ e_smx_state_t simcall_execution_test(smx_activity_t execution)
  * \ingroup simix_process_management
  * \brief Kills all SIMIX processes.
  */
-void simcall_process_killall(int reset_pid)
+void simcall_process_killall()
 {
-  simcall_BODY_process_killall(reset_pid);
+  simcall_BODY_process_killall();
 }
 
 /**

diff --git a/src/simix/popping_accessors.hpp b/src/simix/popping_accessors.hpp
index 106e960..21f3be7 100644 (file)
--- a/src/simix/popping_accessors.hpp
+++ b/src/simix/popping_accessors.hpp
@@ -15,19 +15,6 @@
  */
 
 #include "src/simix/popping_private.hpp"
-static inline int simcall_process_killall__get__reset_pid(smx_simcall_t simcall)
-{
-  return simgrid::simix::unmarshal<int>(simcall->args[0]);
-}
-static inline int simcall_process_killall__getraw__reset_pid(smx_simcall_t simcall)
-{
-  return simgrid::simix::unmarshal_raw<int>(simcall->args[0]);
-}
-static inline void simcall_process_killall__set__reset_pid(smx_simcall_t simcall, int arg)
-{
-  simgrid::simix::marshal<int>(simcall->args[0], arg);
-}
-
 static inline smx_actor_t simcall_process_cleanup__get__process(smx_simcall_t simcall)
 {
   return simgrid::simix::unmarshal<smx_actor_t>(simcall->args[0]);
@@ -1399,7 +1386,7 @@ static inline void simcall_run_blocking__set__code(smx_simcall_t simcall, std::f
 
 /* The prototype of all simcall handlers, automatically generated for you */
 
-XBT_PRIVATE void simcall_HANDLER_process_killall(smx_simcall_t simcall, int reset_pid);
+XBT_PRIVATE void simcall_HANDLER_process_killall(smx_simcall_t simcall);
 XBT_PRIVATE void simcall_HANDLER_process_suspend(smx_simcall_t simcall, smx_actor_t process);
 XBT_PRIVATE void simcall_HANDLER_process_join(smx_simcall_t simcall, smx_actor_t process, double timeout);
 XBT_PRIVATE void simcall_HANDLER_process_sleep(smx_simcall_t simcall, double duration);

diff --git a/src/simix/popping_bodies.cpp b/src/simix/popping_bodies.cpp
index 8bae553..f4638de 100644 (file)
--- a/src/simix/popping_bodies.cpp
+++ b/src/simix/popping_bodies.cpp
@@ -36,11 +36,11 @@ inline static R simcall(e_smx_simcall_t call, T const&... t)
   return simgrid::simix::unmarshal<R>(self->simcall.result);
 }
 
-inline static void simcall_BODY_process_killall(int reset_pid)
+inline static void simcall_BODY_process_killall()
 {
   if (0) /* Go to that function to follow the code flow through the simcall barrier */
-    simcall_HANDLER_process_killall(&SIMIX_process_self()->simcall, reset_pid);
-  return simcall<void, int>(SIMCALL_PROCESS_KILLALL, reset_pid);
+    simcall_HANDLER_process_killall(&SIMIX_process_self()->simcall);
+  return simcall<void>(SIMCALL_PROCESS_KILLALL);
 }
 
 inline static void simcall_BODY_process_cleanup(smx_actor_t process)

diff --git a/src/simix/popping_generated.cpp b/src/simix/popping_generated.cpp
index f24a394..568241c 100644 (file)
--- a/src/simix/popping_generated.cpp
+++ b/src/simix/popping_generated.cpp
@@ -74,7 +74,7 @@ void SIMIX_simcall_handle(smx_simcall_t simcall, int value) {
     return;
   switch (simcall->call) {
 case SIMCALL_PROCESS_KILLALL:
-  simcall_HANDLER_process_killall(simcall, simgrid::simix::unmarshal<int>(simcall->args[0]));
+  simcall_HANDLER_process_killall(simcall);
   SIMIX_simcall_answer(simcall);
   break;
 

diff --git a/src/simix/simcalls.in b/src/simix/simcalls.in
index 82fd939..00bcbac 100644 (file)
--- a/src/simix/simcalls.in
+++ b/src/simix/simcalls.in
@@ -35,7 +35,7 @@
 # Last but not the least, you should declare the new simix call in
 # ./include/simgrid/simix.h (otherwise you will get a warning at compile time)
 
-void process_killall(int reset_pid);
+void process_killall();
 void process_cleanup(smx_actor_t process) [[nohandler]];
 void process_suspend(smx_actor_t process) [[block]];
 int  process_join(smx_actor_t process, double timeout) [[block]];

diff --git a/src/simix/smx_global.cpp b/src/simix/smx_global.cpp
index a5f316f..4ea34c2 100644 (file)
--- a/src/simix/smx_global.cpp
+++ b/src/simix/smx_global.cpp
@@ -282,7 +282,7 @@ void SIMIX_clean()
 #endif
 
   /* Kill all processes (but maestro) */
-  SIMIX_process_killall(simix_global->maestro_process, 1);
+  SIMIX_process_killall(simix_global->maestro_process);
   SIMIX_context_runall();
   SIMIX_process_empty_trash();
 

diff --git a/teshsuite/s4u/pid/pid.cpp b/teshsuite/s4u/pid/pid.cpp
index 552f17c..69b7ff8 100644 (file)
--- a/teshsuite/s4u/pid/pid.cpp
+++ b/teshsuite/s4u/pid/pid.cpp
@@ -42,10 +42,7 @@ int main(int argc, char* argv[])
   simgrid::s4u::Engine e(&argc, argv);
   e.loadPlatform(argv[1]);
 
-  if (argc > 2)
-    simgrid::s4u::Actor::killAll(atoi(argv[2]));
-  else
-    simgrid::s4u::Actor::killAll();
+  simgrid::s4u::Actor::killAll();
 
   simgrid::s4u::Actor::createActor("sendpid", simgrid::s4u::Host::by_name("Tremblay"), sendpid);
   simgrid::s4u::Actor::createActor("sendpid", simgrid::s4u::Host::by_name("Tremblay"), sendpid);

diff --git a/teshsuite/s4u/pid/pid.tesh b/teshsuite/s4u/pid/pid.tesh
index 2c2c1ca..86011db 100644 (file)
--- a/teshsuite/s4u/pid/pid.tesh
+++ b/teshsuite/s4u/pid/pid.tesh
@@ -11,17 +11,3 @@ $ ./pid ${platfdir}/small_platform.xml "--log=root.fmt:[%10.6r]%e(%P@%h)%e%m%n"
 > [  0.001206] (sendpid@Tremblay) Send of pid "3" done.
 > [  0.001206] (killall@Tremblay) Killing process "3".
 > [  0.001206] (sendpid@Tremblay) Process "3" killed.
-
-$ ./pid ${platfdir}/small_platform.xml 2 "--log=root.fmt:[%10.6r]%e(%P@%h)%e%m%n"
-> [  0.000000] (sendpid@Tremblay) Sending pid of "2".
-> [  0.000000] (sendpid@Tremblay) Sending pid of "3".
-> [  0.000000] (sendpid@Tremblay) Sending pid of "4".
-> [  0.000402] (killall@Tremblay) Killing process "2".
-> [  0.000402] (sendpid@Tremblay) Send of pid "2" done.
-> [  0.000402] (sendpid@Tremblay) Process "2" killed.
-> [  0.000804] (sendpid@Tremblay) Send of pid "3" done.
-> [  0.000804] (killall@Tremblay) Killing process "3".
-> [  0.000804] (sendpid@Tremblay) Process "3" killed.
-> [  0.001206] (sendpid@Tremblay) Send of pid "4" done.
-> [  0.001206] (killall@Tremblay) Killing process "4".
-> [  0.001206] (sendpid@Tremblay) Process "4" killed.