Re: oaf time stamp problem

["Troy D . Gillette" <tgillette turbolinux com>]

On Fri, 27 Oct 2000, Elliot Lee wrote:

> On Thu, 26 Oct 2000, Troy D . Gillette wrote:
> 
> > OK, I've pinned it down, but am not sure on one point.
> > 
> > The problem occurs because of the oaf_naming_service. When I call
> > oaf_init() and then oaf_activate(), the oaf child variable gets time stamped
> > and the oaf_naming_service gets added as an active server.
> > 
> > This is fine, except that it is rapidly followed by my own
> > oaf_active_server_register() call. Which usually happens within the same
> > second, and therefore DOESN'T get added to the child's active_servers list.
> > 
> > So any calls to check if that server is active will fail until another server
> > gets registered, causing the list to get updated again.
> 
> Oh yuck, this is a nasty one. 
> 
> Two ways to solve it:
> 	- Use more precise timestamps
> 	- Switch from using a timestamp to using some sort of unique ID
> 	number that will change every time the _active state of a server
> 	in the OD changes.
> 
> The latter solution is totally race-condition-free. Main decider is
> whether we will need actual time info in any other place.
> 
> > My question is... What starts oaf_naming_service?
> 
> It is started & registered as part of oafd startup.

Here's a patch that solves it. Basically I turned the only_if_newer value
for get_active_servers() into an inout type, so that the child stamp in
child_od_update_active() is set directly by the call to
impl_OAF_ObjectDirectory_get_active_servers().

The time_active_changed variable in od-corba.c is initialized to zero when
the OD is created, and incremented for each register or unregister.

This should be safe for up to 2^64 register & unregister calls. ;)


     Troy D. Gillette - Principal Software Developer - TurboLinux, Inc.
_____________________________________.oOo.____________________________________
Labor, n.:
One of the processes by which A acquires property for B.
    -- Ambrose Bierce, "The Devil's Dictionary"



diff -Naur oaf.bak/idl/oaf.idl oaf/idl/oaf.idl
--- oaf.bak/idl/oaf.idl	Thu Oct  5 21:25:32 2000
+++ oaf/idl/oaf.idl	Fri Oct 27 10:58:26 2000
@@ -126,7 +126,7 @@
 			UNREGISTER_NOTIFY_UNUSED /* For factories to use */
 		};
                 ServerInfoListCache get_servers (in CacheTime only_if_newer);
-                ServerStateCache get_active_servers (in CacheTime only_if_newer);
+                ServerStateCache get_active_servers (inout CacheTime only_if_newer);
                 
                 readonly attribute string username, hostname, domain;
 
diff -Naur oaf.bak/oafd/ac-corba.c oaf/oafd/ac-corba.c
--- oaf.bak/oafd/ac-corba.c	Fri Oct 20 03:14:56 2000
+++ oaf/oafd/ac-corba.c	Fri Oct 27 11:01:08 2000
@@ -115,7 +115,7 @@
 	OAF_ServerStateCache *cache;
 
 	cache = OAF_ObjectDirectory_get_active_servers (child->obj,
-							child->time_active_pulled,
+							&(child->time_active_pulled),
 							ev);
 	if (ev->_major != CORBA_NO_EXCEPTION) {
 		child_od_exception (child, ev);
@@ -130,7 +130,6 @@
 
 		child->active_server_list = cache;
 
-		child->time_active_pulled = time (NULL);
 		child->active_servers =
 			g_hash_table_new (g_str_hash, g_str_equal);
 		g_hash_table_freeze (child->active_servers);
diff -Naur oaf.bak/oafd/od-corba.c oaf/oafd/od-corba.c
--- oaf.bak/oafd/od-corba.c	Mon Sep 25 16:36:55 2000
+++ oaf/oafd/od-corba.c	Fri Oct 27 11:00:53 2000
@@ -73,7 +73,7 @@
 
 static OAF_ServerStateCache
 	* impl_OAF_ObjectDirectory_get_active_servers
-	(impl_POA_OAF_ObjectDirectory * servant, OAF_CacheTime only_if_newer,
+	(impl_POA_OAF_ObjectDirectory * servant, OAF_CacheTime *only_if_newer,
 	 CORBA_Environment * ev);
 
 static CORBA_char
@@ -284,6 +284,7 @@
 	newservant->attr_domain = g_strdup (domain);
 	newservant->attr_hostID = oaf_hostname_get ();
 	newservant->by_iid = NULL;
+	newservant->time_active_changed = 0;
 
         newservant->registry_source_directories = g_strsplit (registry_path, ":", -1);
         newservant->registry_directory_mtimes = g_hash_table_new (g_str_hash, g_str_equal);
@@ -313,6 +314,7 @@
 		CORBA_sequence_set_release (&retval->_u.server_list,
 					    CORBA_FALSE);
 	}
+	only_if_newer = servant->time_active_changed;
 
 	return retval;
 }
@@ -333,14 +335,14 @@
 static OAF_ServerStateCache *
 impl_OAF_ObjectDirectory_get_active_servers (impl_POA_OAF_ObjectDirectory *
 					     servant,
-					     OAF_CacheTime only_if_newer,
+					     OAF_CacheTime *only_if_newer,
 					     CORBA_Environment * ev)
 {
 	OAF_ServerStateCache *retval;
 
 	retval = OAF_ServerStateCache__alloc ();
 
-	retval->_d = (only_if_newer < servant->time_active_changed);
+	retval->_d = (*only_if_newer < servant->time_active_changed);
 	if (retval->_d) {
 		StateCollectionInfo sci;
 
@@ -356,6 +358,7 @@
 		CORBA_sequence_set_release (&(retval->_u.active_servers),
 					    CORBA_TRUE);
 	}
+	*only_if_newer = servant->time_active_changed;
 
 	return retval;
 }
@@ -509,7 +512,7 @@
 	g_hash_table_insert (servant->active_servers,
 			     oldobj ? iid : g_strdup (iid),
 			     CORBA_Object_duplicate (obj, ev));
-	servant->time_active_changed = time (NULL);
+	servant->time_active_changed++;
 
 	return OAF_REG_SUCCESS;
 }
@@ -535,5 +538,5 @@
 	g_free (orig_iid);
 	CORBA_Object_release (orig_obj, ev);
 
-	servant->time_active_changed = time (NULL);
+	servant->time_active_changed++;
 }