From 0c931ec14d88197c14b73b33f175534cae3c1977 Mon Sep 17 00:00:00 2001
From: edgrif <edgrif>
Date: Wed, 3 Mar 2004 12:16:46 +0000
Subject: [PATCH] Add new create/destroy routines for better handling of
creation/termination. Sort out the killing of GUI and threads so they are
correct for a ZMap to kill itself. Add new states to ZMap so we can better
control interaction with user and with threads. Each Zmap can now run
multiple threads to contact multiple servers. Make cleanup code more robust
by using state of Zmap and state of threads.
---
src/zmapControl/zmapControl.c | 510 +++++++++++++++++++++++-----------
1 file changed, 355 insertions(+), 155 deletions(-)
diff --git a/src/zmapControl/zmapControl.c b/src/zmapControl/zmapControl.c
index bb893f865..ac5fb5f1f 100755
--- a/src/zmapControl/zmapControl.c
+++ b/src/zmapControl/zmapControl.c
@@ -26,13 +26,14 @@
* the window code and the threaded server code.
* Exported functions: See ZMap.h
* HISTORY:
- * Last edited: Jan 23 11:24 2004 (edgrif)
+ * Last edited: Mar 2 10:25 2004 (edgrif)
* Created: Thu Jul 24 16:06:44 2003 (edgrif)
- * CVS info: $Id: zmapControl.c,v 1.2 2004-01-23 13:28:09 edgrif Exp $
+ * CVS info: $Id: zmapControl.c,v 1.3 2004-03-03 12:16:46 edgrif Exp $
*-------------------------------------------------------------------
*/
#include <gtk/gtk.h>
+#include <ZMap/zmapUtils.h>
#include <ZMap_P.h>
@@ -42,11 +43,21 @@ gboolean zmap_debug_G = TRUE ;
static gint zmapIdleCB(gpointer cb_data) ;
static void zmapWindowCB(void *cb_data, int reason) ;
-static void checkConnection(ZMap zmap) ;
+
+ZMap createZMap(char *sequence, void *app_data, ZMapCallbackFunc destroy_cb, ZMapConfig config) ;
+void destroyZMap(ZMap zmap) ;
+
static void startConnectionChecking(ZMap zmap) ;
static void stopConnectionChecking(ZMap zmap) ;
-static void killGUI(ZMap zmap) ;
+static gboolean checkConnections(ZMap zmap) ;
+
+static void loadDataConnections(ZMap zmap) ;
+
static void killZMap(ZMap zmap) ;
+static void killGUI(ZMap zmap) ;
+static void killConnections(ZMap zmap) ;
+static void destroyConnection(ZMapConnection *connection) ;
+
static gboolean getServer(char *server_string, char **machine, char **port, char **protocol) ;
@@ -57,125 +68,98 @@ static gboolean getServer(char *server_string, char **machine, char **port, char
*/
-/* Trivial at the moment..... */
-ZMap zMapCreate(void *app_data, ZMapCallbackFunc destroy_cb, ZMapConfig config)
+/* Create a new/blank zmap with its window, has no thread connections to databases.
+ * Returns NULL on failure. */
+ZMap zMapCreate(char *sequence, void *app_data, ZMapCallbackFunc destroy_cb, ZMapConfig config)
{
ZMap zmap = NULL ;
- /* GROSS HACK FOR NOW, NEED SOMETHING BETTER LATER... */
- static int zmap_num = 0 ;
-
- zmap = g_new(ZMapStruct, sizeof(ZMapStruct)) ;
- zmap_num++ ;
-
- zmap->zmap_id = g_strdup_printf("%d", zmap_num) ;
+ zmap = createZMap(sequence, app_data, destroy_cb, config) ;
- zmap->state = ZMAP_INIT ;
-
- zmap->sequence = NULL ;
-
- zmap->config = config ;
-
- zmap->connection_list = NULL ;
-
- zmap->app_data = app_data ;
- zmap->destroy_zmap_cb = destroy_cb ;
+ /* Create the zmap window itself. */
+ if (!(zmap->window = zMapWindowCreate(zmap->sequence, zmap->zmap_id, zmapWindowCB, zmap)))
+ {
+ destroyZMap(zmap) ;
+ zmap = NULL ;
+ }
+ else
+ zmap->state = ZMAP_INIT ;
return zmap ;
}
-/* Create a new zmap with its window and a connection to a database via a thread,
+/* Connect a blank ZMap to its databases via threads,
* at this point the ZMap is blank and waiting to be told to load some data. */
-gboolean zMapConnect(ZMap zmap, char *sequence)
+gboolean zMapConnect(ZMap zmap)
{
gboolean result = TRUE ;
char **server_list = NULL ;
- zmap->sequence = sequence ;
-
- /* We need to retrieve the connect data here from the config stuff.... */
- if (result)
+ if (zmap->state != ZMAP_INIT)
{
- if (!(zMapConfigGetServers(zmap->config, &server_list)))
- result = FALSE ;
+ /* Probably we should indicate to caller what the problem was here.... */
+ result = FALSE ;
}
-
-
- /* Set up a connection to the named server. */
- if (result)
+ else
{
- char **next = server_list ;
- int connections = 0 ;
+ /* We need to retrieve the connect data here from the config stuff.... */
+ if (result)
+ {
+ if (!(zMapConfigGetServers(zmap->config, &server_list)))
+ result = FALSE ;
+ }
- /* Error handling is tricky here, if we manage to connect to some servers and not
- * others, what do we do ? THINK ABOUT THIS..... */
- while (result && *next != NULL)
+ /* Set up connections to the named servers. */
+ if (result)
{
- char *machine, *port, *protocol ;
- ZMapConnection connection ;
+ char **next = server_list ;
+ int connections = 0 ;
- if (getServer(*next, &machine, &port, &protocol)
- && (connection = zMapConnCreate(machine, port, protocol, sequence)))
+ /* Error handling is tricky here, if we manage to connect to some servers and not
+ * others, what do we do ? THINK ABOUT THIS..... */
+ while (result && *next != NULL)
{
- zmap->connection_list = g_list_append(zmap->connection_list, connection) ;
- connections++ ;
- }
- else
- {
- gchar *err_msg ;
-
- err_msg = g_strdup_printf("Could not connect to %s protocol server "
- "on %s, port %s", protocol, machine, port) ;
+ char *machine, *port, *protocol ;
+ ZMapConnection connection ;
- zmapGUIShowMsg(err_msg) ;
+ if (getServer(*next, &machine, &port, &protocol)
+ && (connection = zMapConnCreate(machine, port, protocol, zmap->sequence)))
+ {
+ zmap->connection_list = g_list_append(zmap->connection_list, connection) ;
+ connections++ ;
+ }
+ else
+ {
+ gchar *err_msg ;
- g_free(err_msg) ;
- }
+ err_msg = g_strdup_printf("Could not connect to %s protocol server "
+ "on %s, port %s", protocol, machine, port) ;
- next++ ;
- }
+ zmapGUIShowMsg(err_msg) ;
- if (!connections)
- result = FALSE ;
- }
+ g_free(err_msg) ;
+ }
- /* Create the zmap window itself. */
- if (result)
- {
- if (!(zmap->window = zMapWindowCreate(sequence, zmap->zmap_id, zmapWindowCB, zmap)))
- result = FALSE ;
- }
+ next++ ;
+ }
- /* Now set up our idle routine to check the connection. */
- if (result)
- {
- startConnectionChecking(zmap) ;
- }
+ if (!connections)
+ result = FALSE ;
+ }
- /* If everything is ok then we are connected to the server, otherwise signal failure
- * and clean up. */
- if (result)
- {
- zmap->state = ZMAP_CONNECTED ;
- }
- else
- {
- if (zmap->connection_list)
+ /* If everything is ok then set up idle routine to check the connections,
+ * otherwise signal failure and clean up. */
+ if (result)
{
-
- /* OK HERE WE NEED TO CLEAN UP PROPERLY.....NEED A FUNCTION I THINK.... */
-
- /* NOTE, we do a _kill_ here, not a destroy. This just signals the thread to die, it
- * will actually die sometime later and be properly cleaned up by code in
- * zMapManagerCheckConnections() */
- zMapConnKill(zmap->connection) ;
+ zmap->state = ZMAP_RUNNING ;
+ startConnectionChecking(zmap) ;
}
-
- if (zmap->window)
+ else
{
- killGUI(zmap) ;
+ zmap->state = ZMAP_DYING ;
+ killZMap(zmap) ;
}
}
@@ -183,12 +167,17 @@ gboolean zMapConnect(ZMap zmap, char *sequence)
}
-/* Signal that we want data to stick into the ZMap */
+/* Signal threads that we want data to stick into the ZMap */
gboolean zMapLoad(ZMap zmap, char *sequence)
{
gboolean result = TRUE ;
- zMapConnLoadData(zmap->connection) ;
+ if (zmap->state != ZMAP_RUNNING)
+ result = FALSE ;
+ else
+ {
+ loadDataConnections(zmap) ;
+ }
return result ;
}
@@ -207,25 +196,41 @@ gboolean zMapLoad(ZMap zmap, char *sequence)
* */
gboolean zMapReset(ZMap zmap)
{
+ gboolean result = TRUE ;
+
+ if (zmap->state != ZMAP_RUNNING)
+ result = FALSE ;
+ else
+ {
+ zmap->state = ZMAP_RESETTING ;
- /* We need a new windows call to reset the window and blank it. */
+ /* We need a new windows call to reset the window and blank it. */
+
+#ifdef ED_G_NEVER_INCLUDE_THIS_CODE
+ zMapWindowReset(zmap->window) ;
+#endif /* ED_G_NEVER_INCLUDE_THIS_CODE */
+
+ /* We need to destroy the existing thread connection and wait until user loads a new
+ sequence. */
+ killConnections(zmap) ;
+ }
- /* We need to destroy the existing thread connection and start a new one...watch out,
- * remember the destroy will be asynchronous..... */
return TRUE ;
}
+
/*
* A set of accessor functions.
*/
char *zMapGetZMapID(ZMap zmap)
{
- char *id ;
+ char *id = NULL ;
- id = zmap->zmap_id ;
+ if (zmap->state != ZMAP_DYING)
+ id = zmap->zmap_id ;
return id ;
}
@@ -233,9 +238,10 @@ char *zMapGetZMapID(ZMap zmap)
char *zMapGetSequence(ZMap zmap)
{
- char *sequence ;
+ char *sequence = NULL ;
- sequence = zmap->sequence ;
+ if (zmap->state != ZMAP_DYING)
+ sequence = zmap->sequence ;
return sequence ;
}
@@ -243,8 +249,8 @@ char *zMapGetSequence(ZMap zmap)
char *zMapGetZMapStatus(ZMap zmap)
{
/* Array must be kept in synch with ZmapState enum in ZMap_P.h */
- static char *zmapStates[] = {"ZMAP_INIT", "ZMAP_CONNECTED", "ZMAP_LOADING",
- "ZMAP_DISPLAYED", "ZMAP_QUITTING"} ;
+ static char *zmapStates[] = {"ZMAP_INIT", "ZMAP_RUNNING", "ZMAP_RESETTING",
+ "ZMAP_DYING", "ZMAP_DEAD"} ;
char *state ;
state = zmapStates[zmap->state] ;
@@ -253,39 +259,52 @@ char *zMapGetZMapStatus(ZMap zmap)
}
-
-/* Called to kill a zmap window and get the associated thread killed, this latter will be
- * asynchronous. */
+/* Called to kill a zmap window and get the associated threads killed, note that
+ * this call just signals everything to die, its the checkConnections() routine
+ * that really clears up.
+ */
gboolean zMapDestroy(ZMap zmap)
{
- stopConnectionChecking(zmap) ;
-
- killGUI(zmap) ;
-
- /* NOTE, we do a _kill_ here, not a destroy. This just signals the thread to die, it
- * will actually die sometime later...check clean up sequence..... */
- zMapConnKill(zmap->connection) ;
+ if (zmap->state != ZMAP_DYING)
+ {
+ killGUI(zmap) ;
- /* CHECK THAT THIS ALL WORKS...IT SHOULD DO..... */
- g_free(zmap->zmap_id) ;
+ /* If we are resetting then the connections have already being killed. */
+ if (zmap->state != ZMAP_RESETTING)
+ killConnections(zmap) ;
- g_free(zmap) ;
+ /* Must set this as this will prevent any further interaction with the ZMap as
+ * a result of both the ZMap window and the threads dying asynchronously. */
+ zmap->state = ZMAP_DYING ;
+ }
return TRUE ;
}
+/* We could provide an "executive" kill call which just left the threads dangling, a kind
+ * of "kill -9" style thing.... */
+
+
+
+
/* This is really the guts of the code to check what a connection thread is up
* to. Every time the GUI thread has stopped doing things this routine gets called
* so then we check our connection for action..... */
static gint zmapIdleCB(gpointer cb_data)
{
+ gint call_again = 0 ;
ZMap zmap = (ZMap)cb_data ;
- checkConnection(zmap) ;
+ /* Returning a value > 0 tells gtk to call zmapIdleCB again, so if checkConnections() returns
+ * TRUE we ask to be called again. */
+ if (checkConnections(zmap))
+ call_again = 1 ;
+ else
+ call_again = 0 ;
- return 1 ; /* > 0 tells gtk to keep calling zmapIdleCB */
+ return call_again ;
}
@@ -330,16 +349,52 @@ static void zmapWindowCB(void *cb_data, int reason)
-
/*
* ------------------- Internal functions -------------------
*/
+ZMap createZMap(char *sequence, void *app_data, ZMapCallbackFunc destroy_cb, ZMapConfig config)
+{
+ ZMap zmap = NULL ;
+
+ /* GROSS HACK FOR NOW, NEED SOMETHING BETTER LATER, JUST A TACKY ID...... */
+ static int zmap_num = 0 ;
+
+ zmap = g_new(ZMapStruct, sizeof(ZMapStruct)) ;
+
+ zmap_num++ ;
+ zmap->zmap_id = g_strdup_printf("%d", zmap_num) ;
+
+ zmap->sequence = g_strdup(sequence) ;
+
+ zmap->config = config ;
+
+ zmap->connection_list = NULL ;
+
+ zmap->app_data = app_data ;
+ zmap->destroy_zmap_cb = destroy_cb ;
+
+ return zmap ;
+}
+
+
+/* Should only do this after the window and all threads have gone as this is our only handle
+ * to these resources. */
+void destroyZMap(ZMap zmap)
+{
+ g_free(zmap->zmap_id) ;
+
+ g_free(zmap->sequence) ;
+
+ g_free(zmap) ;
+
+ return ;
+}
/* Start and stop the ZMap GTK idle function (gets run when the GUI is doing nothing).
- * */
+ */
static void startConnectionChecking(ZMap zmap)
{
zmap->idle_handle = gtk_idle_add(zmapIdleCB, (gpointer)zmap) ;
@@ -347,6 +402,10 @@ static void startConnectionChecking(ZMap zmap)
return ;
}
+
+/* I think that probably I won't need this most of the time, it could be used to remove the idle
+ * function in a kind of unilateral way as a last resort, otherwise the idle function needs
+ * to cancel itself.... */
static void stopConnectionChecking(ZMap zmap)
{
gtk_idle_remove(zmap->idle_handle) ;
@@ -359,84 +418,210 @@ static void stopConnectionChecking(ZMap zmap)
/* This function checks the status of the connection and checks for any reply and
* then acts on it, it gets called from the ZMap idle function.
+ * If all threads are ok and zmap has not been killed then routine returns TRUE
+ * meaning it wants to be called again, otherwise FALSE.
*
* NOTE that you cannot use a condvar here, if the connection thread signals us using a
* condvar we will probably miss it, that just doesn't work, we have to pole for changes
* and this is possible because this routine is called from the idle function of the GUI.
* */
-static void checkConnection(ZMap zmap)
+static gboolean checkConnections(ZMap zmap)
{
- ZMapThreadReply reply ;
+ gboolean call_again = TRUE ; /* Normally we want to called continuously. */
+ GList* list_item ;
- if (zmap->connection)
+ list_item = g_list_first(zmap->connection_list) ;
+ /* should assert this as never null.... */
+
+ do
{
- gboolean got_value ;
+ ZMapConnection connection ;
+ ZMapThreadReply reply ;
void *data = NULL ;
char *err_msg = NULL ;
+ connection = list_item->data ;
+
+
#ifdef ED_G_NEVER_INCLUDE_THIS_CODE
ZMAP_DEBUG(("GUI: checking connection for thread %x\n",
- zMapConnGetThreadid(zmap->connection))) ;
+ zMapConnGetThreadid(connection))) ;
#endif /* ED_G_NEVER_INCLUDE_THIS_CODE */
+
data = NULL ;
- if (!(got_value = zMapConnGetReplyWithData(zmap->connection, &reply, &data, &err_msg)))
+ err_msg = NULL ;
+ if (!(zMapConnGetReplyWithData(connection, &reply, &data, &err_msg)))
{
- printf("GUI: thread state locked, cannot access it....\n") ;
+ ZMAP_DEBUG(("GUI: thread %x, cannot access reply from thread - %s\n",
+ zMapConnGetThreadid(connection), err_msg)) ;
}
else
{
#ifdef ED_G_NEVER_INCLUDE_THIS_CODE
- ZMAP_DEBUG(("GUI: got state for thread %x, state = %s\n",
- zMapConnGetThreadid(zmap->connection),
- zmapVarGetStringState(reply))) ;
+ ZMAP_DEBUG(("GUI: thread %x, thread reply = %s\n",
+ zMapConnGetThreadid(connection),
+ zMapVarGetReplyString(reply))) ;
#endif /* ED_G_NEVER_INCLUDE_THIS_CODE */
-
+
if (reply == ZMAP_REPLY_WAIT)
{
; /* nothing to do. */
}
else if (reply == ZMAP_REPLY_GOTDATA)
{
- ZMAP_DEBUG(("GUI: got data for thread %x\n",
- zMapConnGetThreadid(zmap->connection))) ;
+ if (zmap->state == ZMAP_RUNNING)
+ {
+ ZMAP_DEBUG(("GUI: thread %x, got data\n",
+ zMapConnGetThreadid(connection))) ;
- zMapConnSetReply(zmap->connection, ZMAP_REPLY_WAIT) ;
+ /* Is this right....????? check my logic here.... */
+ zMapConnSetReply(connection, ZMAP_REPLY_WAIT) ;
- /* Signal the ZMap that there is work to be done. */
- zMapWindowSignalData(zmap->window, data) ;
+ /* Signal the ZMap that there is work to be done. */
+ zMapWindowSignalData(zmap->window, data) ;
+ }
+ else
+ ZMAP_DEBUG(("GUI: thread %x, got data but ZMap state is - %s\n",
+ zMapConnGetThreadid(connection), zMapGetZMapStatus(zmap))) ;
+
}
else if (reply == ZMAP_REPLY_DIED)
{
+ if (err_msg)
+ zmapGUIShowMsg(err_msg) ;
+
+
/* This means the thread has failed for some reason and we should clean up. */
ZMAP_DEBUG(("GUI: thread %x has died so cleaning up....\n",
- zMapConnGetThreadid(zmap->connection))) ;
-
- if (err_msg && *err_msg)
- zmapGUIShowMsg(err_msg) ;
+ zMapConnGetThreadid(connection))) ;
- killZMap(zmap) ;
+ /* We are going to remove an item from the list so better move on from
+ * this item. */
+ list_item = g_list_next(list_item) ;
+ zmap->connection_list = g_list_remove(zmap->connection_list, connection) ;
+
+ destroyConnection(&connection) ;
}
else if (reply == ZMAP_REPLY_CANCELLED)
{
+ /* I'm not sure we need to do anything here as now this loop is "inside" a
+ * zmap, we should already chopping the zmap threads outside of this routine,
+ * so I think logically this cannot happen...???? */
+
/* This means the thread was cancelled so we should clean up..... */
ZMAP_DEBUG(("GUI: thread %x has been cancelled so cleaning up....\n",
- zMapConnGetThreadid(zmap->connection))) ;
+ zMapConnGetThreadid(connection))) ;
- killZMap(zmap) ;
+ /* We are going to remove an item from the list so better move on from
+ * this item. */
+ list_item = g_list_next(list_item) ;
+ zmap->connection_list = g_list_remove(zmap->connection_list, connection) ;
+
+ destroyConnection(&connection) ;
}
}
}
+ while ((list_item = g_list_next(list_item))) ;
- return ;
+
+ /* At this point if there are no threads left we need to examine our state and take action
+ * depending on whether we are dying or threads have died or whatever.... */
+ if (!zmap->connection_list)
+ {
+ if (zmap->state == ZMAP_DYING)
+ {
+ /* zmap was waiting for threads to die, now they have we can free everything and stop. */
+ destroyZMap(zmap) ;
+
+ call_again = FALSE ;
+ }
+ else if (zmap->state == ZMAP_RESETTING)
+ {
+ /* zmap is ok but user has reset it and all threads have died so we need to stop
+ * checking.... */
+ zmap->state = ZMAP_INIT ;
+
+ call_again = FALSE ;
+ }
+ else
+ {
+ /* Threads have died because of their own errors....so what should we do ?
+ * for now we kill the window and then the rest of ZMap..... */
+ zmapGUIShowMsg("Cannot show ZMap because server connections have all died") ;
+
+ killGUI(zmap) ;
+ destroyZMap(zmap) ;
+
+ call_again = FALSE ;
+ }
+ }
+
+ return call_again ;
}
+static void loadDataConnections(ZMap zmap)
+{
+
+ if (zmap->connection_list)
+ {
+ GList* list_item ;
+
+ list_item = g_list_first(zmap->connection_list) ;
+
+ do
+ {
+ ZMapConnection connection ;
+
+ connection = list_item->data ;
+
+ /* ERROR HANDLING..... */
+ zMapConnLoadData(connection) ;
+
+ } while ((list_item = g_list_next(list_item))) ;
+ }
+
+ return ;
+}
+
/* SOME ORDER ISSUES NEED TO BE ATTENDED TO HERE...WHEN SHOULD THE IDLE ROUTINE BE REMOVED ? */
+/* destroys the window if this has not happened yet and then destroys the slave threads
+ * if there are any.
+ */
+static void killZMap(ZMap zmap)
+{
+ /* precise order is not straight forward...bound to be some race conditions here but...
+ *
+ * - should inactivate GUI as this should be cheap and prevents further interactions, needs
+ * to inactivate ZMap _and_ controlling GUI.
+ * - then should kill the threads so we can then wait until they have all died _before_
+ * freeing up data etc. etc.
+ * - then we should stop checking the threads once they have died.
+ * - then GUI should disappear so user cannot interact with it anymore.
+ * Then we should kill the threads */
+
+
+ if (zmap->window != NULL)
+ {
+ killGUI(zmap) ;
+ }
+
+ /* NOTE, we do a _kill_ here, not a destroy. This just signals the thread to die, it
+ * will actually die sometime later...check clean up sequence..... */
+ if (zmap->connection_list != NULL)
+ {
+ killConnections(zmap) ;
+ }
+
+ return ;
+}
+
+
/* Calls the control window callback to remove any reference to the zmap and then destroys
* the actual zmap itself.
*
@@ -458,24 +643,39 @@ static void killGUI(ZMap zmap)
}
-
-/* destroys the window if this has not happened yet and then destroys the slave thread
- * control block.
- */
-static void killZMap(ZMap zmap)
+static void killConnections(ZMap zmap)
{
- stopConnectionChecking(zmap) ;
+ GList* list_item ;
- if (zmap->window != NULL)
+ list_item = g_list_first(zmap->connection_list) ;
+
+ do
{
- killGUI(zmap) ;
+ ZMapConnection connection ;
+
+ connection = list_item->data ;
+
+ /* NOTE, we do a _kill_ here, not a destroy. This just signals the thread to die, it
+ * will actually die sometime later. */
+ zMapConnKill(connection) ;
}
+ while ((list_item = g_list_next(list_item))) ;
+
+ return ;
+}
+
+
+static void destroyConnection(ZMapConnection *connection)
+{
+ zMapConnDestroy(*connection) ;
+
+ *connection = NULL ;
- zMapConnDestroy(zmap->connection) ;
-
return ;
}
+
+
/* In some ways this code should really be in zmapConfig...maybe....
* We are expecting server_string to be of the form:
*
@@ -497,8 +697,8 @@ static gboolean getServer(char *server_string, char **machine, char **port, char
if ((tokens = g_strsplit(server_string, " ", 3)))
{
*machine = tokens[0] ;
- *port = (++tokens)[0] ;
- *protocol = (++tokens)[0] ;
+ *port = tokens[1] ;
+ *protocol = tokens[2] ;
/* NOTE WELL the minimalistic error handling here.... */
if (machine && port && protocol)
--
GitLab