[omniORB] issues in name service

[Jiang Wei]

Naming Service Specification Version 1.3 say:
2.3.1.2 next_n
... ...
? A zero value of how_many is illegal and raises a BAD_PARAM system
exception.

2.3.2 Garbage Collection of Iterators
Clients that create iterators but never call destroy can cause an
implementation to
permanently run out of resources. To protect itself against this
scenario, an
implementation is free to destroy an iterator object at any time without
warning, using
whatever algorithm it considers appropriate to choose iterators for
destruction. In order
to be robust in the presence of garbage collection, clients should be
written to expect
OBJECT_NOT_EXIST from calls to an iterator and handle this exception
gracefully.


I tried to make a patch :

Index: src/appl/omniNames/BindingIterator_i.h
===================================================================
RCS file:
/cvsroot/omniorb/omni/src/appl/omniNames/Attic/BindingIterator_i.h,v
retrieving revision 1.7.2.2
diff -u -r1.7.2.2 BindingIterator_i.h
--- src/appl/omniNames/BindingIterator_i.h 5 Feb 2008 12:47:14 -0000 1.7.2.2
+++ src/appl/omniNames/BindingIterator_i.h 3 Jun 2009 08:56:28 -0000
@@ -34,6 +34,7 @@
# endif
#endif

+#define MAX_ITERATOR_NUM 4096

class BindingIterator_i : public POA_CosNaming::BindingIterator,
public PortableServer::RefCountServantBase
@@ -44,6 +45,27 @@
BindingIterator_i(PortableServer::POA_ptr poa, CosNaming::BindingList* l)
: list(l)
{
+ omni_mutex_lock lock(iters_mutex);
+
+ my_pos = next_pos;
+ while (iterators[my_pos]) {
+ my_pos = (my_pos+1) % MAX_ITERATOR_NUM;
+ if (my_pos == next_pos) {
+ break;
+ }
+ }
+ if (my_pos == next_pos)
+ next_pos = (next_pos+1) % MAX_ITERATOR_NUM;
+
+ BindingIterator_i* old = iterators[my_pos];
+ if (old) {
+ PortableServer::ObjectId_var id = names_poa->servant_to_id(old);
+ names_poa->deactivate_object(id);
+ old->_remove_ref();
+ }
+
+ this->_add_ref();
+ iterators[my_pos] = this;
PortableServer::ObjectId_var id = poa->activate_object(this);
}

@@ -65,6 +87,12 @@
}

CORBA::Boolean next_n(CORBA::ULong how_many, CosNaming::BindingList_out
bl) {
+ // A zero value of how_many is illegal and raises a BAD_PARAM system
+ // exception.
+
+ if(how_many == 0) {
+ throw CORBA::BAD_PARAM(0, CORBA::COMPLETED_NO);
+ }

//
// What we do here is return the current list to the caller, shortening
@@ -84,21 +112,35 @@

bl->length(how_many);

- if (how_many == 0)
+ if (how_many == 0) {
+ destroy();
return 0;
+ }
else
return 1;
}

void destroy(void) {
// remember the destructor for an object should never be called explicitly.
- PortableServer::ObjectId_var id = names_poa->servant_to_id(this);
- names_poa->deactivate_object(id);
+ omni_mutex_lock lock(iters_mutex);
+ if (iterators[my_pos] == this) {
+ PortableServer::ObjectId_var id = names_poa->servant_to_id(this);
+ names_poa->deactivate_object(id);
+ iterators[my_pos] = 0;
+ _remove_ref();
+ } else {
+ throw CORBA::OBJECT_NOT_EXIST(0, CORBA::COMPLETED_NO);
+ }
+
}

private:

- CosNaming::BindingList* list;
+ CosNaming::BindingList* list;
+ int my_pos;
+ static BindingIterator_i* iterators[MAX_ITERATOR_NUM];
+ static int next_pos;
+ static omni_mutex iters_mutex;

// remember the destructor for an object should never be called explicitly.
~BindingIterator_i() {
Index: src/appl/omniNames/NamingContext_i.cc
===================================================================
RCS file:
/cvsroot/omniorb/omni/src/appl/omniNames/Attic/NamingContext_i.cc,v
retrieving revision 1.13.2.5
diff -u -r1.13.2.5 NamingContext_i.cc
--- src/appl/omniNames/NamingContext_i.cc 6 May 2009 16:16:00 -0000 1.13.2.5
+++ src/appl/omniNames/NamingContext_i.cc 3 Jun 2009 08:56:28 -0000
@@ -490,6 +490,11 @@
return;
}

+ if (how_many == 0) {
+ bl = new CosNaming::BindingList;
+ return;
+ }
+
DB(cerr << " calling next_n()" << endl);
bi->next_n(how_many, bl);
}
@@ -549,3 +554,8 @@
CosNaming::Name_var name = omniURI::stringToName(sn);
return resolve(name);
}
+
+BindingIterator_i* BindingIterator_i::iterators[MAX_ITERATOR_NUM];
+int BindingIterator_i::next_pos;
+omni_mutex BindingIterator_i::iters_mutex;
+
Index: src/appl/utils/nameclt/nameclt.cc
===================================================================
RCS file: /cvsroot/omniorb/omni/src/appl/utils/nameclt/Attic/nameclt.cc,v
retrieving revision 1.20.2.3
diff -u -r1.20.2.3 nameclt.cc
--- src/appl/utils/nameclt/nameclt.cc 8 Jun 2005 09:35:37 -0000 1.20.2.3
+++ src/appl/utils/nameclt/nameclt.cc 3 Jun 2009 08:56:37 -0000
@@ -239,8 +239,10 @@

cout << endl;
}
-
- bi->destroy();
+ try {
+ bi->destroy();
+ } catch (CORBA::OBJECT_NOT_EXIST&) {
+ }
goto done;
}