Date: Monday, December 11, 2006 @ 19:14:45
Author: gilles
Path: /cvsroot/carob/carob
Modified: include/Connection.hpp (1.75 -> 1.76) include/ControllerPool.hpp
(1.9 -> 1.10) include/ControllerStateChangedCallback.hpp (1.3 ->
1.4) src/Connection.cpp (1.98 -> 1.99) src/ControllerInfo.cpp
(1.5 -> 1.6) src/ControllerPool.cpp (1.14 -> 1.15)
src/ControllerStateChangedCallback.cpp (1.4 -> 1.5)
Put getController() and connect() together inside a ControllerPool-locked block
to avoid concurrent
controller add or delete during connection initConnection is now called
initConnectionToNextController
and holds this synchronized block
Fixed leak when ControllerInfo::connect() fails: the socket pointer was not
deleted. Had to create a
'unregister socket' function in controller pool and callback to avoid double
destructions
--------------------------------------------+
include/Connection.hpp | 12 +++++---
include/ControllerPool.hpp | 17 +++++++++++-
include/ControllerStateChangedCallback.hpp | 7 ++++-
src/Connection.cpp | 37 ++++++++++++++-------------
src/ControllerInfo.cpp | 17 ++++++++++--
src/ControllerPool.cpp | 7 +++++
src/ControllerStateChangedCallback.cpp | 21 +++++++++++++++
7 files changed, 92 insertions(+), 26 deletions(-)
Index: carob/include/Connection.hpp
diff -u carob/include/Connection.hpp:1.75 carob/include/Connection.hpp:1.76
--- carob/include/Connection.hpp:1.75 Thu Nov 30 17:47:49 2006
+++ carob/include/Connection.hpp Mon Dec 11 19:14:45 2006
@@ -147,7 +147,8 @@
* Creates a new connection and connects to a controller with the given
* parameters
* @param prms controller, base, user, etc. to use for connecting
- * @throw DriverException if the ConnectPolicy given in parameters is unknown
+ * @throw DriverException if the ConnectPolicy given in connection parameters
+ * is unknown or if the ConnectPolicy could not be created
* @throw ConnectionException if the requested controller could not be
* reached
* @throw AuthenticationException if a login error occured, or if an
@@ -373,20 +374,21 @@
protected:
/**
- * Starts a connection by sending all connection/authentication informations.
+ * Starts a connection by retrieving a new controller to connect to from the
+ * pool and sends all connection/authentication informations to it.
* Warning: this function only sends the connection infos, but there is no
* guaranty that we are connected afterwards. [EMAIL PROTECTED]
#finalizeConnect()} will
* do the job of checking and finishing connection establishment.
* @return true if the connection initialization succeeds
- * @throw DriverException if the connect policy is NULL
+ * @throw NoMoreControllerException if there is no alive controller left
* @throw ConnectionException if the requested controller could not be
* reached
* @throw AuthenticationException if a login error occured, or if an
* IO exception occured during authentication (premature close of
* connection)
*/
- void initConnection()
- throw (DriverException, ConnectionException,
+ void initConnectionToNextController()
+ throw (NoMoreControllerException,
ConnectionException,
AuthenticationException, UnexpectedException);
/**
* Reads the controller acknowledgements and misc parameters that finish
Index: carob/include/ControllerPool.hpp
diff -u carob/include/ControllerPool.hpp:1.9
carob/include/ControllerPool.hpp:1.10
--- carob/include/ControllerPool.hpp:1.9 Fri Dec 8 16:07:10 2006
+++ carob/include/ControllerPool.hpp Mon Dec 11 19:14:45 2006
@@ -24,7 +24,6 @@
#include "ControllerInfo.hpp"
#include "CarobException.hpp"
#include "CriticalSection.hpp"
-
#include <vector>
#include <map>
@@ -98,6 +97,16 @@
JavaSocket* socket);
/**
+ * Unregisters the given socket from the callback so it can no more be killed
+ * or deleted. To be used when socket creation or connection failed
+ *
+ * @param controller the controller to which the socket is connected
+ * @param socketFd the socket file descriptor to unregister
+ */
+ void unRegisterSocket(const ControllerInfo&
controller,
+ JavaSocket* socket);
+
+ /**
* Tell the watcher that a failure has been detected on the given
controller.<br>
* This function should be called when a connection error occurs on the given
* controller
@@ -125,6 +134,12 @@
void controllerUp(const ControllerInfo& controller);
/**
+ * Gets a reference to the lock object used in this class
+ * @see void Connection::initConnection()
+ */
+ CriticalSection& getLock() { return pool_CS; }
+
+ /**
* Display useful info in a string
*/
operator std::wstring();
Index: carob/include/ControllerStateChangedCallback.hpp
diff -u carob/include/ControllerStateChangedCallback.hpp:1.3
carob/include/ControllerStateChangedCallback.hpp:1.4
--- carob/include/ControllerStateChangedCallback.hpp:1.3 Mon Dec 4
15:13:48 2006
+++ carob/include/ControllerStateChangedCallback.hpp Mon Dec 11 19:14:45 2006
@@ -83,7 +83,12 @@
void onControllerUp(const ControllerInfo& ctrl);
/** Adds the given socket to the list of monitored sockets for given
controller */
void registerSocket(const ControllerInfo& ctrl, JavaSocket* socketFd);
-
+ /**
+ * Removes the given socket from the list of monitored sockets for a given
+ * controller. This function should be called when socket creation or
+ * connection failed
+ */
+ void unRegisterSocket(const ControllerInfo& ctrl, JavaSocket* socketFd);
private:
/** The list of controllers with their set of sockets */
std::map<ControllerInfo, std::vector<JavaSocket*> > controllers_and_sockets;
Index: carob/src/Connection.cpp
diff -u carob/src/Connection.cpp:1.98 carob/src/Connection.cpp:1.99
--- carob/src/Connection.cpp:1.98 Fri Dec 8 18:02:08 2006
+++ carob/src/Connection.cpp Mon Dec 11 19:14:45 2006
@@ -92,10 +92,8 @@
{
try
{
- connected_controller = controller_pool.getController();
-
// Do the authentication stuff
- initConnection();
+ initConnectionToNextController();
// receive ack and other params
finalizeConnect();
isClosed = false;
@@ -114,8 +112,6 @@
{
//Do some clean up
delete driverSocketPtr; driverSocketPtr = NULL;
- //suspect failing controller and try next one
- controller_pool.forceControllerDown(connected_controller);
}
catch (NoMoreControllerException& nmce)
{
@@ -155,23 +151,29 @@
socket<<static_cast<int32_t>(command);
}
-void Connection::initConnection()
- throw (DriverException, ConnectionException, AuthenticationException,
- UnexpectedException)
+void Connection::initConnectionToNextController()
+ throw (NoMoreControllerException, ConnectionException,
+ AuthenticationException, UnexpectedException)
{
- wstring fctName(L"Connection::initConnection");
+ wstring fctName(L"Connection::initConnectionToNextController");
bool protocolVersionSend = false,
dbNameSend = false,
userNameSend = false,
userPassSend = false;
- if (isInfoEnabled())
- logInfo(fctName, L"Authenticating with controller "
- + static_cast<wstring>(connected_controller));
try
{
- //Here is the connection protocol...
- // 1. connect to the controller
- driverSocketPtr = connected_controller.connect(controller_pool);
+ // This must lock the pool synchro to avoid controller list operations
+ // between getController() and connect() calls
+ {
+ LockScope(&controller_pool.getLock());
+ connected_controller = controller_pool.getController();
+ if (isInfoEnabled())
+ logInfo(fctName, L"Authenticating with controller "
+ + static_cast<wstring>(connected_controller));
+ //Here is the connection protocol...
+ // 1. connect to the controller
+ driverSocketPtr = connected_controller.connect(controller_pool);
+ }
// 2. sent the Protocol version information
*driverSocketPtr<<ProtocolVersion;
protocolVersionSend = true;
@@ -191,6 +193,8 @@
+ L" (" + connExcpt.description() + L").";
if (isErrorEnabled())
logError(fctName, msg);
+ //suspect failing controller
+ controller_pool.forceControllerDown(connected_controller);
throw ConnectionException(msg);
}
catch (SocketIOException sockIOExcpt)
@@ -1260,8 +1264,7 @@
{
if (isDebugEnabled())
logDebug(fctName, L"Getting a new controller from the list...");
- connected_controller = controller_pool.getController();
- initConnection();
+ initConnectionToNextController();
finalizeConnect();
if (isInfoEnabled())
logInfo(fctName, L"Connection succeded");
Index: carob/src/ControllerInfo.cpp
diff -u carob/src/ControllerInfo.cpp:1.5 carob/src/ControllerInfo.cpp:1.6
--- carob/src/ControllerInfo.cpp:1.5 Mon Dec 4 15:13:48 2006
+++ carob/src/ControllerInfo.cpp Mon Dec 11 19:14:45 2006
@@ -210,10 +210,19 @@
if (isWarnEnabled())
logWarn(fctName, L"IP v6 host detected. This feature is untested");
}
- socketPtr->create(family);
+ try
+ {
+ socketPtr->create(family);
+ }
+ catch (ConnectionException ce)
+ {
+ delete socketPtr;
+ freeaddrinfo(addr);
+ throw;
+ }
// register the socket to our policy so it can be killed during connection
pool.registerSocket(*this, socketPtr);
-
+
if (isDebugEnabled())
logDebug(fctName, L"Connecting...");
// if we got multiple addresses, we will try to connect to all of these (in
@@ -236,6 +245,8 @@
}
catch (ConnectionException ce)
{
+ pool.unRegisterSocket(*this, socketPtr);
+ delete socketPtr;
freeaddrinfo(addr);
throw ce;
}
@@ -244,6 +255,8 @@
addrIter = addrIter->ai_next;
}
// could not connect to any of the addresses
+ pool.unRegisterSocket(*this, socketPtr);
+ delete socketPtr;
freeaddrinfo(addr);
throw ConnectionException(L"Unable to connect. Last error code was "
+ toUserString(errno));
Index: carob/src/ControllerPool.cpp
diff -u carob/src/ControllerPool.cpp:1.14 carob/src/ControllerPool.cpp:1.15
--- carob/src/ControllerPool.cpp:1.14 Fri Dec 8 16:07:10 2006
+++ carob/src/ControllerPool.cpp Mon Dec 11 19:14:45 2006
@@ -92,6 +92,13 @@
callback_ptr->registerSocket(controller, socket);
}
+void AbstractControllerPool::unRegisterSocket(const ControllerInfo& controller,
+ JavaSocket* socket)
+{
+ LockScope ls(&pool_CS);
+ callback_ptr->unRegisterSocket(controller, socket);
+}
+
void AbstractControllerPool::forceControllerDown(const ControllerInfo&
controller)
{
LockScope ls(&pool_CS);
Index: carob/src/ControllerStateChangedCallback.cpp
diff -u carob/src/ControllerStateChangedCallback.cpp:1.4
carob/src/ControllerStateChangedCallback.cpp:1.5
--- carob/src/ControllerStateChangedCallback.cpp:1.4 Mon Dec 4 15:13:48 2006
+++ carob/src/ControllerStateChangedCallback.cpp Mon Dec 11 19:14:45 2006
@@ -62,6 +62,27 @@
controllers_and_sockets[ctrl] = sockets;
}
+void SocketKillerCallback::unRegisterSocket(const ControllerInfo& ctrl,
+ JavaSocket* s)
+{
+ LockScope ls(&controllers_and_sockets_CS);
+ map<ControllerInfo, vector<JavaSocket*> >::iterator ctrlIter =
+ controllers_and_sockets.find(ctrl);
+ if (ctrlIter != controllers_and_sockets.end())
+ {
+ vector<JavaSocket*> sockets = ctrlIter->second;
+ vector<JavaSocket*>::iterator socketIter = sockets.begin();
+ while (socketIter != sockets.end())
+ {
+ if (*socketIter == s)
+ socketIter = sockets.erase(socketIter);
+ else
+ socketIter++;
+ }
+ controllers_and_sockets[ctrl] = sockets;
+ }
+}
+
void SocketKillerCallback::onControllerDown(const ControllerInfo& ctrl)
{
wstring fctName(L"SocketKillerCallback::OnControllerDown");
_______________________________________________
Carob-commits mailing list
[email protected]
https://forge.continuent.org/mailman/listinfo/carob-commits
