Fix: dispatching any action causes actmon/actlog to crash with SIGSEGV

.gitignore

@@ -128,6 +128,7 @@ trigger.opts
 
 # bootstrap output files
 LabView/Makefile.in
+actions/testing/Makefile.in
 configure
 ChangeLog
 MANIFEST.MF

actions/actlogp.h

@@ -203,7 +203,6 @@ static void MessageAst(void *dummy __attribute__((unused)), char *reply)
   }
   free(event->msg);
   free(event);
-  CheckIn(0);
 }
 
 inline static void _EventUpdate(LinkedEvent *event)

actions/actmon.c

@@ -66,7 +66,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <ctype.h>
 #include <mdsplus/mdsplus.h>
 static void Exit(Widget w, int *tag, XtPointer callback_data);
-static void MessageAst();
 static void EventUpdate();
 static void Phase(LinkedEvent *event);
 static void Dispatched(LinkedEvent *event);

actions/testing/Makefile.am

@@ -0,0 +1,28 @@
+
+include @top_builddir@/Makefile.inc
+include ../../testing/testing.am
+
+
+## ////////////////////////////////////////////////////////////////////////// ##
+## // TESTS  //////////////////////////////////////////////////////////////// ##
+## ////////////////////////////////////////////////////////////////////////// ##
+
+TEST_EXTENSIONS = .sh
+
+TESTS = \
+  ServerShrTest.sh
+
+# Files produced by tests that must be purged
+#
+MOSTLYCLEANFILES = \
+	./*.log ./*.err\
+	*_*.characteristics* *_*.datafile* *_*.tree*
+## ////////////////////////////////////////////////////////////////////////// ##
+## // TARGETS  ////////////////////////////////////////////////////////////// ##
+## ////////////////////////////////////////////////////////////////////////// ##
+
+
+#clean-local: clean-local-tests
+
+check_PROGRAMS =
+check_SCRIPTS  = $(TESTS)

actions/testing/ServerShrTest.sh

@@ -0,0 +1,61 @@
+#!/bin/bash
+
+srcdir=$(readlink -f $(dirname ${0}))
+
+# See testing/ports.csv
+mon_port=8021
+act_port=8022
+dis_port=8023
+if [ ! -z $TEST_PORT_OFFET ]; then
+  mon_port=$((mon_port + TEST_PORT_OFFSET))
+  act_port=$((act_port + TEST_PORT_OFFSET))
+  dis_port=$((dis_port + TEST_PORT_OFFSET))
+fi
+
+export MONITOR_SERVER=localhost:$mon_port
+export ACTION_SERVER=localhost:$act_port
+export DISPATCH_SERVER=localhost:$dis_port
+
+
+if [ "$OS" == "windows" ]; then
+  MDSIP="wine mdsip"
+  MDSTCL="wine mdstcl"
+  ACTLOG="wine actlog"
+  DIFF_Z="-Z"
+else
+  MDSIP="mdsip"
+  MDSTCL="mdstcl"
+  ACTLOG="actlog"
+  DIFF_Z=""
+fi
+
+export distest_path=$PWD
+
+mkdir -p $distest_path
+
+$MDSIP -p ${mon_port} -s -h $MDSPLUS_DIR/testing/mdsip.hosts > server_mon.log 2> server_mon.err & # Monitor server
+mon_pid=$!
+$MDSIP -p ${act_port} -s -h $MDSPLUS_DIR/testing/mdsip.hosts > server_act.log 2> server_act.err & # Action server
+act_pid=$!
+$MDSIP -p ${dis_port} -s -h $MDSPLUS_DIR/testing/mdsip.hosts > server_dis.log 2> server_dis.err & # Dispatch server
+dis_pid=$!
+$ACTLOG -monitor ${MONITOR_SERVER} > actlog.log 2> actlog.err &
+log_pid=$!
+
+$MDSTCL @${srcdir}/build_tree.tcl
+
+$MDSTCL @${srcdir}/test_action.tcl
+
+kill -SIGKILL $mon_pid $act_pid $dis_pid $log_pid
+
+# Cut time info
+cat actlog.log | cut -d' ' -f2- > actlog_cut.log
+
+# Check output of actlog against reference
+if diff ${DIFF_Z} actlog_cut.log ${srcdir}/actlog.ans; then
+  echo "PASS: actlog"
+  exit 0
+else
+  echo "FAIL: actlog"
+  exit 1
+fi

actions/testing/actlog.ans

@@ -0,0 +1,5 @@
+           1 NEW SHOT   ******************************************** ******************** DISTEST
+           1 PHASE      ******************************************** ******************** INITIALIZATION            
+           1 DISPATCHED                                              ACTION_SERVER        \DISTEST::TOP:ACT01
+           1 DOING                                                   ACTION_SERVER        \DISTEST::TOP:ACT01
+           1 DONE                                                    ACTION_SERVER        \DISTEST::TOP:ACT01

actions/testing/build_tree.tcl

@@ -0,0 +1,7 @@
+edit distest /shot=1 /new
+add node act01 /usage=action
+write
+put/extend ACT01
+Build_Action(Build_Dispatch(2,"ACTION_SERVER","INIT",10,*),BUILD_FUNCTION(BUILTIN_OPCODE("COMMA"),BUILD_FUNCTION(BUILTIN_OPCODE("WRITE"),*,"Test action"),1))
+
+close

actions/testing/test_action.tcl

@@ -0,0 +1,6 @@
+dispatch /command /server=DISPATCH_SERVER set tree distest /shot=1
+wait 1
+dispatch /command /server=DISPATCH_SERVER dispatch/build/monitor=MONITOR_SERVER
+wait 1
+dispatch /command /server=DISPATCH_SERVER dispatch/phase/monitor=MONITOR_SERVER init
+wait 1

configure.ac

@@ -1375,6 +1375,7 @@ AC_OUTPUT(
   Makefile.inc
   _include/_mdsversion.h
   actions/Makefile
+  actions/testing/Makefile
   camshr/Makefile
   ccl/Makefile
   d3dshr/Makefile

include/servershr.h

@@ -28,17 +28,17 @@ EXPORT extern int ServerBuildDispatchTable(char *wildcard, char *monitor_name,
                                            void **table);
 EXPORT extern int ServerCloseTrees(char *server);
 EXPORT extern int ServerCreatePulse(int *id, char *server, char *tree, int shot,
-                                    void (*ast)(), void *astprm, int *retstatus,
+                                    void (*ast)(void *, char *), void *astprm, int *retstatus,
                                     pthread_rwlock_t *lock,
                                     void (*before_ast)());
 EXPORT extern int ServerDispatchAction(int *id, char *server, char *tree,
-                                       int shot, int nid, void (*ast)(),
+                                       int shot, int nid, void (*ast)(void *, char *),
                                        void *astprm, int *retstatus,
                                        pthread_rwlock_t *lock, int *socket,
                                        void (*before_ast)());
 EXPORT extern int ServerDispatchClose(void *vtable);
 EXPORT extern int ServerDispatchCommand(int *id, char *server, char *cli,
-                                        char *command, void (*ast)(),
+                                        char *command, void (*ast)(void *, char *),
                                         void *astprm, int *retstatus,
                                         pthread_rwlock_t *lock,
                                         void (*before_ast)());
@@ -51,7 +51,7 @@ EXPORT extern int ServerDispatchPhase(int *id, void *vtable, char *phasenam,
                                       const char *monitor);
 EXPORT extern int ServerFailedEssential(void *vtable, int reset);
 EXPORT extern char *ServerFindServers(void **ctx, char *wild_match);
-EXPORT extern int ServerMonitorCheckin(char *server, void (*ast)(),
+EXPORT extern int ServerMonitorCheckin(char *server, void (*ast)(void *, char *),
                                        void *astparam);
 EXPORT extern int ServerSetLogging(char *server, char logging_mode);
 EXPORT extern int ServerStartServer(char *server);

servershr/Client.h

@@ -108,7 +108,7 @@ static void Client_cleanup_jobs(Client *c, fd_set *fdactive)
     Job *j = Job_pop_by_conid(conid);
     if (j)
     {
-      Job_callback_done(j, ServerPATH_DOWN, FALSE);
+      Job_callback_done(j, ServerPATH_DOWN, NULL, FALSE);
       free(j);
     }
     else
@@ -198,7 +198,7 @@ static void Client_do_message(Client *c, fd_set *fdactive)
       j = Job_get_by_jobid(MonJob);
     if (j)
     {
-      Job_callback_done(j, status, TRUE);
+      Job_callback_done(j, status, msg, TRUE);
     }
     else
     {

servershr/Job.h

@@ -11,7 +11,7 @@ typedef struct job
   int conid;
   int *retstatus;
   pthread_rwlock_t *lock;
-  void (*callback_done)();
+  void (*callback_done)(void *, char*);
   void (*callback_before)();
   void *callback_param;
   pthread_cond_t *cond;
@@ -30,7 +30,7 @@ static Job *Jobs = NULL;
 static int MonJob = -1;
 
 static Job *newJob(int conid, int *retstatus, pthread_rwlock_t *lock,
-                   void (*callback_done)(void *),
+                   void (*callback_done)(void *, char *),
                    void *callback_param,
                    void (*callback_before)(void *))
 {
@@ -71,7 +71,7 @@ static void Job_pop(Job *job)
 static int Job_register(int *msgid,
                         int conid, int *retstatus, pthread_rwlock_t *lock,
                         void *callback_param,
-                        void (*callback_done)(void *),
+                        void (*callback_done)(void *, char *),
                         void (*callback_before)(void *))
 {
   Job *j = newJob(conid, retstatus, lock, callback_param, callback_done, callback_before);
@@ -107,10 +107,10 @@ static void Job_callback_before(Job *job)
 }
 
 /// returns  true if job was popped
-static int Job_callback_done(Job *j, int status, int remove)
+static int Job_callback_done(Job *j, int status, char *msg, int remove)
 {
   MDSDBG(JOB_PRI " status=%d, remove=%d", JOB_VAR(j), status, remove);
-  void (*callback_done)(void *);
+  void (*callback_done)(void *, char *);
   const int is_mon = j->jobid == MonJob;
   if (j->lock)
     pthread_rwlock_wrlock(j->lock);
@@ -122,7 +122,7 @@ static int Job_callback_done(Job *j, int status, int remove)
   if (j->lock)
     pthread_rwlock_unlock(j->lock);
   if (callback_done)
-    callback_done(j->callback_param);
+    callback_done(j->callback_param, msg);
   /**** If job has a condition, RemoveJob will not remove it. ***/
   if (remove && !is_mon)
   {
@@ -258,7 +258,7 @@ static void Job_cleanup(int status, int jobid)
   do
   {
     MDSDBG(JOB_PRI " done", JOB_VAR(j));
-    Job_callback_done(j, status, FALSE);
+    Job_callback_done(j, status, NULL, FALSE);
     free(j);
     j = Job_pop_by_conid(conid);
   } while (j);

servershr/ServerCreatePulse.c

@@ -59,7 +59,7 @@ dsc$descriptor *tree, int *shot, void (*ast)(), int astprm, int *netid, void
 #include "servershrp.h"
 
 EXPORT int ServerCreatePulse(int *id, char *server, char *tree, int shot,
-                             void (*ast)(), void *astprm, int *retstatus,
+                             void (*ast)(void *, char *), void *astprm, int *retstatus,
                              pthread_rwlock_t *lock, void (*before_ast)())
 {
   struct descrip p1, p2;

servershr/ServerDispatchAction.c

@@ -59,7 +59,7 @@ dsc$descriptor *tree, int *shot, int *nid, void (*ast)(), int astprm, int
 #include "servershrp.h"
 
 EXPORT int ServerDispatchAction(int *id, char *server, char *tree, int shot,
-                                int nid, void (*ast)(), void *astprm,
+                                int nid, void (*ast)(void *, char*), void *astprm,
                                 int *retstatus, pthread_rwlock_t *lock,
                                 int *socket, void (*before_ast)())
 {

servershr/ServerDispatchCommand.c

@@ -59,7 +59,7 @@ int *netid, void (*link_down)(), void (*before_ast)())
 #include "servershrp.h"
 
 EXPORT int ServerDispatchCommand(int *id, char *server, char *cli,
-                                 char *command, void (*ast)(), void *astprm,
+                                 char *command, void (*ast)(void *, char *), void *astprm,
                                  int *retstatus, pthread_rwlock_t *lock,
                                  void (*before_ast)())
 {

servershr/ServerDispatchPhase.c

@@ -86,7 +86,7 @@ extern int ProgLoc;
 
 static void dispatch(int idx);
 static void send_monitor(int mode, int idx);
-static void action_done(intptr_t idx);
+static void action_done(void *arg, char *msg);
 static void action_done_action_locked(int idx);
 static void action_done_action_unlocked(int idx);
 static void before(int idx);
@@ -723,7 +723,7 @@ static void action_done_action_unlocked(int idx)
         UNLOCK_ACTION(cidx, ad_fte);
         WRLOCK_ACTION(cidx, ad_fte);
         actions[cidx].status = ServerINVALID_DEPENDENCY;
-        action_done(cidx);
+        action_done((void *)(intptr_t)cidx, NULL);
         UNLOCK_ACTION(cidx, ad_fte);
         action_done_action_unlocked(cidx);
       }
@@ -836,18 +836,20 @@ static void action_done_thread()
   pthread_cleanup_pop(1);
 }
 
-static void action_done(intptr_t i)
+static void action_done(void *arg, char *dummy __attribute__((unused)))
 {
   INIT_STATUS;
   static pthread_t thread;
+  intptr_t i = (intptr_t) arg;
   action_done_push(i); /***** must be done before starting thread ****/
   CONDITION_START_THREAD(&ActionDoneRunningC, thread, , action_done_thread, NULL);
   if (STATUS_NOT_OK)
     perror("action_done: pthread creation failed");
 }
 #else
-static inline void action_done(intptr_t i)
+static inline void action_done(void *arg, char *dummy __attribute__((unused)))
 {
+  intptr_t i = (intptr_t) arg;
   return action_done_do(i);
 }
 #endif

servershr/ServerMonitorCheckin.c

@@ -101,7 +101,7 @@ static inline int using_events(char *server, void (*ast)(), void *astprm)
   return yesno;
 }
 
-EXPORT int ServerMonitorCheckin(char *server, void (*ast)(), void *astprm)
+EXPORT int ServerMonitorCheckin(char *server, void (*ast)(void *, char *), void *astprm)
 {
   if (using_events(server, ast, astprm))
     return MDSplusSUCCESS;