Date: Thursday, March 1, 2007 @ 12:36:03
Author: marc
Path: /cvsroot/carob/carob
Modified: include/BufferedSocket.hpp (1.5 -> 1.6)
include/ControllerPool.hpp (1.15 -> 1.16)
include/DriverSocket.hpp (1.21 -> 1.22) include/JavaSocket.hpp
(1.46 -> 1.47) src/Connection.cpp (1.114 -> 1.115)
src/ControllerInfo.cpp (1.15 -> 1.16) src/ControllerPool.cpp
(1.23 -> 1.24)
CAROB-133 fix: hid DriverSocket* new & delete inside ControllerPool so
we cannot forget (de-)registration. DriverSocket now holds a private
copy of ControllerInfo to simplify interfaces.
----------------------------+
include/BufferedSocket.hpp | 8 ++++----
include/ControllerPool.hpp | 10 +++++-----
include/DriverSocket.hpp | 17 +++++++++++++++--
include/JavaSocket.hpp | 18 +++++++++---------
src/Connection.cpp | 21 +++++++++++++++++----
src/ControllerInfo.cpp | 15 ++++++---------
src/ControllerPool.cpp | 14 ++++++++------
7 files changed, 64 insertions(+), 39 deletions(-)
Index: carob/include/BufferedSocket.hpp
diff -u carob/include/BufferedSocket.hpp:1.5
carob/include/BufferedSocket.hpp:1.6
--- carob/include/BufferedSocket.hpp:1.5 Wed Feb 14 17:39:29 2007
+++ carob/include/BufferedSocket.hpp Thu Mar 1 12:36:03 2007
@@ -97,15 +97,15 @@
{
public:
/**
- * Empty constructor
- */
- BufferedSocket() throw (CodecException) : JavaSocket() {} ;
- /**
* Sends the buffered write data to the socket
*/
void flush() const throw (SocketIOException,
UnexpectedException);
protected:
/**
+ * Empty constructor
+ */
+ BufferedSocket() throw (CodecException) : JavaSocket() {} ;
+ /**
* Substitute for recv. Waits for incomming data by calling pollOnSingleFd
* and loops until full length has been received or an error occured. If
* shutdown() is called during the loop, throws a SocketIOException to inform
Index: carob/include/ControllerPool.hpp
diff -u carob/include/ControllerPool.hpp:1.15
carob/include/ControllerPool.hpp:1.16
--- carob/include/ControllerPool.hpp:1.15 Wed Feb 28 10:53:27 2007
+++ carob/include/ControllerPool.hpp Thu Mar 1 12:36:03 2007
@@ -21,6 +21,7 @@
#ifndef CONTROLLERPOOL_H_
#define CONTROLLERPOOL_H_
+#include "DriverSocket.hpp"
#include "ControllerInfo.hpp"
#include "CarobException.hpp"
#include "CriticalSection.hpp"
@@ -55,7 +56,7 @@
class SocketKillerCallback;
class JavaSocket;
-/** Holds a controller and its last vdb failure */
+/** Holds a vdb member and its last failure */
class ControllerAndVdbState
{
public:
@@ -137,8 +138,8 @@
* @param controller the controller to which the socket is connected
* @param socket the socket to register
*/
- void registerSocket(const ControllerInfo& controller,
- JavaSocket* socket);
+ DriverSocket* newRegisteredSocket(const ControllerInfo&
controller)
+ throw (CodecException);
/**
* Unregisters the given socket from the callback so it can no more be killed
@@ -147,8 +148,7 @@
* @param controller the controller to which the socket is connected
* @param socket the socket to unregister
*/
- void unRegisterSocket(const ControllerInfo&
controller,
- JavaSocket* socket);
+ void deleteRegisteredSocket(DriverSocket* socket);
/**
* Tell the watcher that a failure has been detected on the given
controller.<br>
Index: carob/include/DriverSocket.hpp
diff -u carob/include/DriverSocket.hpp:1.21 carob/include/DriverSocket.hpp:1.22
--- carob/include/DriverSocket.hpp:1.21 Tue Jan 30 11:55:32 2007
+++ carob/include/DriverSocket.hpp Thu Mar 1 12:36:03 2007
@@ -22,6 +22,8 @@
#ifndef _DRIVERSOCKET_H_
#define _DRIVERSOCKET_H_
+#include "ControllerInfo.hpp"
+
#include "JavaSocket.hpp"
#include "BufferedSocket.hpp"
@@ -43,19 +45,30 @@
: public BufferedSocket
#endif
{
-public:
+
+ // to avoid missing (de-)registrations, only the pool is allowed to
+ // create/delete us. See CAROB-133.
+ friend class AbstractControllerPool;
+private:
/**
* Empty constructor
* @throws ConnectionException if the connection fails
*/
- DriverSocket() throw (CodecException, UnexpectedException)
+ DriverSocket(const ControllerInfo& ctrl) throw (CodecException,
UnexpectedException)
#ifdef CAROB_UNBUFFERED_SOCKET
: JavaSocket()
#else
: BufferedSocket()
#endif
+ , controller(ctrl)
{}
+ ~DriverSocket() { };
+
+public:
+ /** The controller we are (trying to) be connected to. */
+ const ControllerInfo controller;
+ // Hold our own (and cheap) copy to avoid thinking about lifecycles
/**
* Writes a string to the socket according to the controller protocol
* FIXME: we don't support string longer than 64K once UTF8 encoded.
Index: carob/include/JavaSocket.hpp
diff -u carob/include/JavaSocket.hpp:1.46 carob/include/JavaSocket.hpp:1.47
--- carob/include/JavaSocket.hpp:1.46 Wed Feb 14 17:39:29 2007
+++ carob/include/JavaSocket.hpp Thu Mar 1 12:36:03 2007
@@ -48,15 +48,6 @@
{
public:
/**
- * Default constructor - No connection here, just creates and empty socket
- */
- JavaSocket() throw (CodecException);
- /**
- * Destructor for disconnection - closes the socket.
- * @throws SocketIOException
- */
- virtual ~JavaSocket();
- /**
* Gives socket validity status.
* @return true if the socket is valid (has been created) so it can be
* connected
@@ -193,6 +184,15 @@
/** true cancels all input and connections attempts */
bool canceled;
/**
+ * Default constructor - No connection here, just creates and empty socket
+ */
+ JavaSocket() throw (CodecException);
+ /**
+ * Destructor for disconnection - closes the socket.
+ * @throws SocketIOException
+ */
+ virtual ~JavaSocket();
+ /**
* Substitute for recv. Waits for incomming data by calling pollOnSingleFd
* and loops until full length has been received or an erro occured. If
* shutdown() is called during the loop, throws a SocketIOException to inform
Index: carob/src/Connection.cpp
diff -u carob/src/Connection.cpp:1.114 carob/src/Connection.cpp:1.115
--- carob/src/Connection.cpp:1.114 Mon Feb 26 18:28:23 2007
+++ carob/src/Connection.cpp Thu Mar 1 12:36:03 2007
@@ -126,21 +126,33 @@
logError(fctName, ce.description());
// Clean all: we are in constructor, destructor won't be called !
- delete driverSocketPtr; driverSocketPtr = NULL;
+ if (driverSocketPtr)
+ {
+ controller_pool.deleteRegisteredSocket(driverSocketPtr);
+ driverSocketPtr = NULL;
+ }
throw;
}
catch (const std::exception& se)
{
logFatal(fctName, StaticCodecs::fromASCII(se.what()));
- delete driverSocketPtr; driverSocketPtr = NULL;
+ if (driverSocketPtr)
+ {
+ controller_pool.deleteRegisteredSocket(driverSocketPtr);
+ driverSocketPtr = NULL;
+ }
throw;
}
catch (...)
{
logFatal(fctName, L"unknown exception type");
- delete driverSocketPtr; driverSocketPtr = NULL;
+ if (driverSocketPtr)
+ {
+ controller_pool.deleteRegisteredSocket(driverSocketPtr);
+ driverSocketPtr = NULL;
+ }
throw;
}
@@ -321,6 +333,7 @@
// try to do the job twice
try
{
+ logInfo(fctName, L"closing corrupted connection because of" +
reason.description());
this->closeSocket();
}
catch (const SocketIOException& sioe)
@@ -416,7 +429,7 @@
}
// Now we can call the destructor without slowing down concurrent calls
// The system socket closing is done in this destructor
- delete toDelete;
+ controller_pool.deleteRegisteredSocket(toDelete);
}
void Connection::setAutoCommit(bool autoCommitPrm)
Index: carob/src/ControllerInfo.cpp
diff -u carob/src/ControllerInfo.cpp:1.15 carob/src/ControllerInfo.cpp:1.16
--- carob/src/ControllerInfo.cpp:1.15 Thu Jan 25 16:10:52 2007
+++ carob/src/ControllerInfo.cpp Thu Mar 1 12:36:03 2007
@@ -130,7 +130,8 @@
DriverSocket* socketPtr;
try {
- socketPtr = new DriverSocket();
+ // register the socket to our policy so it can be killed during connection
+ socketPtr = pool.newRegisteredSocket(*this);
} catch (CodecException ce) {
std::wostringstream woss;
woss << ce;
@@ -152,11 +153,9 @@
}
catch (ConnectionException ce)
{
- delete socketPtr;
+ pool.deleteRegisteredSocket(socketPtr);
throw;
}
- // register the socket to our policy so it can be killed during connection
- pool.registerSocket(*this, socketPtr);
if (isDebugEnabled())
logDebug(fctName, L"Connecting...");
@@ -173,16 +172,14 @@
}
catch (ConnectionException ce)
{
- pool.unRegisterSocket(*this, socketPtr);
- delete socketPtr;
+ pool.deleteRegisteredSocket(socketPtr);
throw ce;
}
if (isDebugEnabled())
logDebug(fctName, L"Could not connect to " + static_cast<wstring>(*this));
- // could not connect to any of the addresses
- pool.unRegisterSocket(*this, socketPtr);
- delete socketPtr;
+ // could not connect
+ pool.deleteRegisteredSocket(socketPtr);
throw ConnectionException(L"Connection failed");
}
Index: carob/src/ControllerPool.cpp
diff -u carob/src/ControllerPool.cpp:1.23 carob/src/ControllerPool.cpp:1.24
--- carob/src/ControllerPool.cpp:1.23 Wed Feb 28 10:53:27 2007
+++ carob/src/ControllerPool.cpp Thu Mar 1 12:36:03 2007
@@ -22,7 +22,7 @@
#include "ControllerPool.hpp"
#include "ControllerWatcher.hpp"
#include "ControllerStateChangedCallback.hpp"
-#include "JavaSocket.hpp"
+#include "DriverSocket.hpp"
#include "Connection.hpp"
#include "ConnectionParameters.hpp"
@@ -121,18 +121,20 @@
return alive_controllers.size();
}
-void AbstractControllerPool::registerSocket(const ControllerInfo& controller,
- JavaSocket* socket)
+DriverSocket *AbstractControllerPool::newRegisteredSocket(const
ControllerInfo& controller)
+ throw (CodecException)
{
LockScope ls(&pool_CS);
+ DriverSocket *socket = new DriverSocket(controller);
callback_ptr->registerSocket(controller, socket);
+ return socket;
}
-void AbstractControllerPool::unRegisterSocket(const ControllerInfo& controller,
- JavaSocket* socket)
+void AbstractControllerPool::deleteRegisteredSocket(DriverSocket *socket)
{
LockScope ls(&pool_CS);
- callback_ptr->unRegisterSocket(controller, socket);
+ callback_ptr->unRegisterSocket(socket->controller, socket);
+ delete socket;
}
void AbstractControllerPool::forceControllerDown(const ControllerInfo&
controller)
_______________________________________________
Carob-commits mailing list
[email protected]
https://forge.continuent.org/mailman/listinfo/carob-commits
