http://bugzilla.zeroc.com/bugzilla/show_bug.cgi?id=1722,

    http://bugzilla.zeroc.com/bugzilla/show_bug.cgi?id=1721,
    http://bugzilla.zeroc.com/bugzilla/show_bug.cgi?id=1926

9 files changed, 72 insertions, 66 deletions

diff --git a/cpp/CHANGES b/cpp/CHANGES
index e7499144b71..a84e4bf7350 100644
--- a/cpp/CHANGES
+++ b/cpp/CHANGES
@@ -1,6 +1,15 @@
 Changes since version 3.1.1
 ---------------------------
 
+- Deprecated Ice.UseEventLog. Windows services should no longer use
+  this property! Also fixed a bug with the windows event logger which
+  would cause hangs if the event log became full.
+
+- Under Windows Ice Services (Ice::Service) always use the windows
+  event logger unless initData is otherwise configured. Installing the
+  service will now create the appropriate registry keys, and uninstalling
+  the service will remove them.
+
 - createObjectAdapter() and propertyToProxy() print warnings if
   unknown properties are found during creation of the adapter or
   proxy. These warnings can be disabled by setting the property
diff --git a/cpp/include/Ice/Service.h b/cpp/include/Ice/Service.h
index 1f8b9ccc283..5dc971f4877 100755
--- a/cpp/include/Ice/Service.h
+++ b/cpp/include/Ice/Service.h
@@ -115,12 +115,13 @@ public:
     //
     // Installs a Win32 service.
     //
-    int installService(const std::string&, const std::string&, const std::string&, const std::vector<std::string>&);
+    int installService(bool, const std::string&, const std::string&, const std::string&,
+                       const std::vector<std::string>&);
 
     //
     // Uninstalls a Win32 service.
     //
-    int uninstallService(const std::string&);
+    int uninstallService(bool, const std::string&);
 
     //
     // Starts a Win32 service. The argument vector is passed to the
@@ -133,6 +134,8 @@ public:
     //
     int stopService(const std::string&);
 
+    static void setModuleHandle(HMODULE);
+
 #else
 
     //
diff --git a/cpp/install/windows/icegridnode.cfg b/cpp/install/windows/icegridnode.cfg
index d75213fb83c..6b65ef8eec9 100644
--- a/cpp/install/windows/icegridnode.cfg
+++ b/cpp/install/windows/icegridnode.cfg
@@ -41,8 +41,6 @@ IceGrid.Node.Output=C:\Documents and Settings\Local Service\Application Data\Zer
 #IceGrid.Node.RedirectErrToOut=1

 

 #

-# Logging to the Windows Event log

+# Trace properties.

 #

-Ice.UseEventLog=1

-Ice.ProgramName=icegridnode (SampleIceGrid node1)

 IceGrid.Node.Trace.Replica=2

diff --git a/cpp/install/windows/icegridregistry.cfg b/cpp/install/windows/icegridregistry.cfg
index d4145520f39..2950f59fc0a 100644
--- a/cpp/install/windows/icegridregistry.cfg
+++ b/cpp/install/windows/icegridregistry.cfg
@@ -49,9 +49,7 @@ IceGrid.Registry.AdminPermissionsVerifier=SampleIceGrid/NullPermissionsVerifier
 IceGrid.Registry.DefaultTemplates=C:\Ice-@ver@\config\templates.xml

 

 #

-# Logging to the Windows Event log

+# Trace properties.

 #

-Ice.UseEventLog=1

-Ice.ProgramName=icegridregistry (SampleIceGrid Master)

 IceGrid.Registry.Trace.Node=1

 IceGrid.Registry.Trace.Replica=1

diff --git a/cpp/src/Ice/DLLMain.cpp b/cpp/src/Ice/DLLMain.cpp
index dab9d417bb4..c4c72acd827 100755
--- a/cpp/src/Ice/DLLMain.cpp
+++ b/cpp/src/Ice/DLLMain.cpp
@@ -9,6 +9,7 @@
 
 #include <Ice/EventLoggerI.h>
 #include <Ice/ImplicitContextI.h>
+#include <Ice/Service.h>
 
 extern "C" BOOL WINAPI _CRT_INIT(HINSTANCE, DWORD, LPVOID);
 
@@ -27,10 +28,10 @@ ice_DLL_Main(HINSTANCE hDLL, DWORD reason, LPVOID reserved)
         return FALSE;
     }
 #endif
-
     if(reason == DLL_PROCESS_ATTACH)
     {
         Ice::EventLoggerI::setModuleHandle(hDLL);
+        Ice::Service::setModuleHandle(hDLL);
     }
     else if(reason == DLL_THREAD_DETACH)
     {
diff --git a/cpp/src/Ice/EventLoggerI.cpp b/cpp/src/Ice/EventLoggerI.cpp
index 23e90e98c79..b52e6981624 100755
--- a/cpp/src/Ice/EventLoggerI.cpp
+++ b/cpp/src/Ice/EventLoggerI.cpp
@@ -10,14 +10,19 @@
 #include <Ice/EventLoggerI.h>
 #include <Ice/EventLoggerMsg.h>
 #include <Ice/LocalException.h>
+#include <Ice/Network.h> // For errorToString
+#include <IceUtil/StaticMutex.h>
 
 using namespace std;
 
-HMODULE Ice::EventLoggerI::_module = NULL;
+HMODULE Ice::EventLoggerI::_module = 0;
 
-Ice::EventLoggerI::EventLoggerI(const string& appName) : 
-    _appName(appName), _source(NULL)
+static IceUtil::StaticMutex outputMutex = ICE_STATIC_MUTEX_INITIALIZER;
+
+Ice::EventLoggerI::EventLoggerI(const string& theAppName) : 
+    _source(0)
 {
+    string appName = theAppName;
     if(appName.empty())
     {
         InitializationException ex(__FILE__, __LINE__);
@@ -31,47 +36,27 @@ Ice::EventLoggerI::EventLoggerI(const string& appName) :
     // The application name cannot contain backslashes.
     //
     string::size_type pos = 0;
-    while((pos = _appName.find('\\', pos)) != string::npos)
+    while((pos = appName.find('\\', pos)) != string::npos)
     {
-        _appName[pos] = '/';
+        appName[pos] = '/';
     }
-    string key = "SYSTEM\\CurrentControlSet\\Services\\EventLog\\Application\\" + _appName;
+    string key = "SYSTEM\\CurrentControlSet\\Services\\EventLog\\Application\\" + appName;
 
     HKEY hKey;
     DWORD d;
-    LONG err;
 
     //
-    // Try to create the key or gain full key access and set the values
+    // Try to create the key and set the values.
     //
-        
-    bool setValues = true;
-    err = RegCreateKeyEx(HKEY_LOCAL_MACHINE, key.c_str(), 0, "REG_SZ", REG_OPTION_NON_VOLATILE, KEY_ALL_ACCESS, 0,
+    LONG err = RegCreateKeyEx(HKEY_LOCAL_MACHINE, key.c_str(), 0, "REG_SZ", REG_OPTION_NON_VOLATILE, KEY_ALL_ACCESS, 0,
                          &hKey, &d);
-    if(err != ERROR_SUCCESS)
-    {
-        //
-        // If the key exist and I can read it, that's good enough ... we hope the values are set properly.
-        //
-        if(RegOpenKeyEx(HKEY_LOCAL_MACHINE, key.c_str(), 0, KEY_READ, &hKey) == ERROR_SUCCESS)
-        {
-            setValues = false;
-        }
-        else
-        {
-            SyscallException ex(__FILE__, __LINE__);
-            ex.error = err;
-            throw ex;
-        }
-    }
-     
-    if(setValues)
+    if(err == ERROR_SUCCESS)
     {
         //
         // Get the filename of this DLL.
         //
         char path[_MAX_PATH];
-        assert(_module != NULL);
+        assert(_module != 0);
         if(!GetModuleFileName(_module, path, _MAX_PATH))
         {
             RegCloseKey(hKey);
@@ -85,7 +70,7 @@ Ice::EventLoggerI::EventLoggerI(const string& appName) :
         // "EventMessageFile" key should contain the path to this DLL.
         //
         err = RegSetValueEx(hKey, "EventMessageFile", 0, REG_EXPAND_SZ, 
-                            (unsigned char*)path, static_cast<DWORD>(strlen(path) + 1));
+                            reinterpret_cast<unsigned char*>(path), static_cast<DWORD>(strlen(path) + 1));
         if(err != ERROR_SUCCESS)
         {
             RegCloseKey(hKey);
@@ -98,7 +83,8 @@ Ice::EventLoggerI::EventLoggerI(const string& appName) :
         // The "TypesSupported" key indicates the supported event types.
         //
         DWORD typesSupported = EVENTLOG_ERROR_TYPE | EVENTLOG_WARNING_TYPE | EVENTLOG_INFORMATION_TYPE;
-        err = RegSetValueEx(hKey, "TypesSupported", 0, REG_DWORD, (unsigned char*)&typesSupported, sizeof(typesSupported));
+        err = RegSetValueEx(hKey, "TypesSupported", 0, REG_DWORD, reinterpret_cast<unsigned char*>(&typesSupported),
+                            sizeof(typesSupported));
         if(err != ERROR_SUCCESS)
         {
             RegCloseKey(hKey);
@@ -106,15 +92,15 @@ Ice::EventLoggerI::EventLoggerI(const string& appName) :
             ex.error = err;
             throw ex;
         }
+        RegCloseKey(hKey);
     }
 
-    RegCloseKey(hKey);
 
     //
     // The event source must match the registry key.
     //
-    _source = RegisterEventSource(NULL, _appName.c_str());
-    if(_source == NULL)
+    _source = RegisterEventSource(0, appName.c_str());
+    if(_source == 0)
     {
         SyscallException ex(__FILE__, __LINE__);
         ex.error = GetLastError();
@@ -124,10 +110,8 @@ Ice::EventLoggerI::EventLoggerI(const string& appName) :
 
 Ice::EventLoggerI::~EventLoggerI()
 {
-    if(_source != NULL)
-    {
-        DeregisterEventSource(_source);
-    }
+    assert(_source != 0);
+    DeregisterEventSource(_source);
 }
 
 void
@@ -135,11 +119,10 @@ Ice::EventLoggerI::print(const string& message)
 {
     const char* str[1];
     str[0] = message.c_str();
-    if(!ReportEvent(_source, EVENTLOG_INFORMATION_TYPE, 0, EVENT_LOGGER_MSG, NULL, 1, 0, str, NULL))
+    if(!ReportEvent(_source, EVENTLOG_INFORMATION_TYPE, 0, EVENT_LOGGER_MSG, 0, 1, 0, str, 0))
     {
-        SyscallException ex(__FILE__, __LINE__);
-        ex.error = GetLastError();
-        throw ex;
+        IceUtil::StaticMutex::Lock sync(outputMutex);
+        cerr << "ReportEvent failed `" << IceInternal::errorToString(GetLastError()) << "':\n" << message << endl;
     }
 }
 
@@ -156,11 +139,10 @@ Ice::EventLoggerI::trace(const string& category, const string& message)
 
     const char* str[1];
     str[0] = s.c_str();
-    if(!ReportEvent(_source, EVENTLOG_INFORMATION_TYPE, 0, EVENT_LOGGER_MSG, NULL, 1, 0, str, NULL))
+    if(!ReportEvent(_source, EVENTLOG_INFORMATION_TYPE, 0, EVENT_LOGGER_MSG, 0, 1, 0, str, 0))
     {
-        SyscallException ex(__FILE__, __LINE__);
-        ex.error = GetLastError();
-        throw ex;
+        IceUtil::StaticMutex::Lock sync(outputMutex);
+        cerr << "ReportEvent failed `" << IceInternal::errorToString(GetLastError()) << "':\n" << message << endl;
     }
 }
 
@@ -169,11 +151,10 @@ Ice::EventLoggerI::warning(const string& message)
 {
     const char* str[1];
     str[0] = message.c_str();
-    if(!ReportEvent(_source, EVENTLOG_WARNING_TYPE, 0, EVENT_LOGGER_MSG, NULL, 1, 0, str, NULL))
+    if(!ReportEvent(_source, EVENTLOG_WARNING_TYPE, 0, EVENT_LOGGER_MSG, 0, 1, 0, str, 0))
     {
-        SyscallException ex(__FILE__, __LINE__);
-        ex.error = GetLastError();
-        throw ex;
+        IceUtil::StaticMutex::Lock sync(outputMutex);
+        cerr << "ReportEvent failed `" << IceInternal::errorToString(GetLastError()) << "':\n" << message << endl;
     }
 }
 
@@ -182,11 +163,10 @@ Ice::EventLoggerI::error(const string& message)
 {
     const char* str[1];
     str[0] = message.c_str();
-    if(!ReportEvent(_source, EVENTLOG_ERROR_TYPE, 0, EVENT_LOGGER_MSG, NULL, 1, 0, str, NULL))
+    if(!ReportEvent(_source, EVENTLOG_ERROR_TYPE, 0, EVENT_LOGGER_MSG, 0, 1, 0, str, 0))
     {
-        SyscallException ex(__FILE__, __LINE__);
-        ex.error = GetLastError();
-        throw ex;
+        IceUtil::StaticMutex::Lock sync(outputMutex);
+        cerr << "ReportEvent failed `" << IceInternal::errorToString(GetLastError()) << "':\n" << message << endl;
     }
 }
 
diff --git a/cpp/src/Ice/EventLoggerI.h b/cpp/src/Ice/EventLoggerI.h
index ea3b33a0934..0975ed7c21a 100755
--- a/cpp/src/Ice/EventLoggerI.h
+++ b/cpp/src/Ice/EventLoggerI.h
@@ -31,7 +31,6 @@ public:
 
 private:
 
-    std::string _appName;
     HANDLE _source;
     static HMODULE _module;
 };
diff --git a/cpp/src/Ice/Instance.cpp b/cpp/src/Ice/Instance.cpp
index ecb2f466bd2..0ce5c17b853 100644
--- a/cpp/src/Ice/Instance.cpp
+++ b/cpp/src/Ice/Instance.cpp
@@ -583,6 +583,10 @@ IceInternal::Instance::Instance(const CommunicatorPtr& communicator, const Initi
             if(_initData.properties->getPropertyAsInt("Ice.UseEventLog") > 0)
             {
                 _initData.logger = new EventLoggerI(_initData.properties->getProperty("Ice.ProgramName"));
+                //
+                // DEPRECATED PROPERTY: Ice.UseEventLog is deprecated.
+                //
+                _initData.logger->warning("Ice.UseEventLog has been deprecated.");
             }
 #else
             if(_initData.properties->getPropertyAsInt("Ice.UseSyslog") > 0)
diff --git a/cpp/src/Ice/Service.cpp b/cpp/src/Ice/Service.cpp
index a764b81e652..114be889729 100755
--- a/cpp/src/Ice/Service.cpp
+++ b/cpp/src/Ice/Service.cpp
@@ -228,8 +228,10 @@ public:
     {
         const char* str[1];
         str[0] = message.c_str();
+        //
         // We ignore any failures from ReportEvent since there isn't
         // anything we can do about it.
+        //
         ReportEvent(_source, EVENTLOG_INFORMATION_TYPE, 0, EVENT_LOGGER_MSG, 0, 1, 0, str, 0);
     }
 
@@ -246,6 +248,10 @@ public:
 
         const char* str[1];
         str[0] = s.c_str();
+        //
+        // We ignore any failures from ReportEvent since there isn't
+        // anything we can do about it.
+        //
         ReportEvent(_source, EVENTLOG_INFORMATION_TYPE, 0, EVENT_LOGGER_MSG, 0, 1, 0, str, 0);
     }
 
@@ -254,6 +260,10 @@ public:
     {
         const char* str[1];
         str[0] = message.c_str();
+        //
+        // We ignore any failures from ReportEvent since there isn't
+        // anything we can do about it.
+        //
         ReportEvent(_source, EVENTLOG_WARNING_TYPE, 0, EVENT_LOGGER_MSG, 0, 1, 0, str, 0);
     }
 
@@ -262,6 +272,10 @@ public:
     {
         const char* str[1];
         str[0] = message.c_str();
+        //
+        // We ignore any failures from ReportEvent since there isn't
+        // anything we can do about it.
+        //
         ReportEvent(_source, EVENTLOG_ERROR_TYPE, 0, EVENT_LOGGER_MSG, 0, 1, 0, str, 0);
     }
 
@@ -277,7 +291,7 @@ private:
     mangleService(string name)
     {
         //
-        // The application name cannot contain backslashes.
+        // The service name cannot contain backslashes.
         //
         string::size_type pos = 0;
         while((pos = name.find('\\', pos)) != string::npos)