[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
Re: New update: User authentication, DB connection pool functionalities.
sergey@total-knowledge.com wrote:
> Index: libui/UuServlet.h
> ===================================================================
> --- libui/UuServlet.h (revision 0)
> +++ libui/UuServlet.h (revision 0)
> @@ -0,0 +1,67 @@
> +#ifndef UUSERVLET_H
> +#define UUSERVLET_H
> +
> +#include <servlet/ServletConfig.h>
> +#include <servlet/HttpServlet.h>
> +#include <servlet/HttpServletRequest.h>
> +#include <servlet/HttpServletResponse.h>
> +#include <servlet/RequestDispatcher.h>
> +#include <servlet/GenericServlet.h>
> +
> +#include <string>
> +#include <boost/shared_ptr.hpp>
> +#include <boost/lexical_cast.hpp>
> +#include <stdexcept>
> +
> +#include "../libdm/User.h"
> +
> +/**
> + * class UuServlet
> + * This is base class of all UU servlets (both CSP and standard servlets). It
> + * imlements request handling functions (doGet, doPost, etc), which derived
> + * servlets should not touch,
> + * and provides basic login & ACL management.
> + */
> +
> +class UuServlet : public servlet::HttpServlet
> +{
> +public:
> + UuServlet();
> + virtual ~UuServlet ( );
> +
> + typedef boost::shared_ptr<void> setAttribute_t;
> + typedef boost::shared_ptr<void> getAttribute_t;
>
Useless typedefs again?
> + typedef boost::shared_ptr<User> user_t;
> +
> + public:
> + /**
> + * Returns true if the request already has valid associated session, and that
> + * session contains valid User object
> + * @return bool
> + * @param req Request to check
> + */
> + bool isUserLoggedIn (servlet::HttpServletRequest& req) const;
> +
> + /**
> + * Redirects browser to login page
> + * @param resp send redirect through this response object
> + */
> + void redirectToLogin (servlet::HttpServletResponse& resp);
> +
> + /**
> + * @return User
> + * @param req
> + */
> + User getCurrentUser (servlet::HttpServletRequest& req);
>
Do not pass objects by value. Use shared_ptr, or, when no reference counting
is needed, just references or pointers.
> +
> + /**
> + * @return UuDbPool <<<<<<<<<<<<------------ more comments are due.
> + */
> +
> + UuDbPool* getDbPool ();
> +
> + void init();
> + void destroy();
> +};
> +
> +#endif // UUSERVLET_H
>
>
> Index: libui/UuServlet.cpp
> ===================================================================
> --- libui/UuServlet.cpp (revision 0)
> +++ libui/UuServlet.cpp (revision 0)
> @@ -0,0 +1,115 @@
> +#include "UuServlet.h"
> +
> +using namespace std;
> +using namespace servlet;
> +using boost::lexical_cast;
> +
> +UuServlet::UuServlet () { }
> +UuServlet::~UuServlet ( ) { }
> +
> +/**
> + * Returns true if the request already has valid associated session, and that
> + * session contains valid User object
> + * @return bool
> + * @param req Request to check
>
Eh?? I think this is misplaced.
> + */
> +
> +void UuServlet::init()
> +{
> +
> + string strDsn = getServletConfig().getInitParameter("DSN");
> + string strUid = getServletConfig().getInitParameter("UID");
> + string strPwd = getServletConfig().getInitParameter("PWD");
> + string strDb = getServletConfig().getInitParameter("DB");
> + string strNumCon = getServletConfig().getInitParameter("num_connections");
> + string strTimeout = getServletConfig().getInitParameter("timeout");
>
Do you expect to provide these parameters for _each_ servlet?
Do you plan to actually instantiate UuServlet? This is wrong.
What you should do is
1. Use App-level parameters (these can be retrieved from ServletContext)
2. provide virtual uuInit() method, for derived servlets to override
3. call it from UuServlet::init()
4. Derived servlets should not override init(). (Not sure if there is a
syntactic
way of making sure of that)
> + if(strDsn.empty())
> + strDsn="PostgreSQL";
> + if(strUid.empty())
> + strUid="sergey";
> + if(strPwd.empty())
> + strPwd="";
> + if(strDb.empty())
> + strDb="uu";
> + if(strNumCon.empty())
> + strNumCon="5";
> + if(strTimeout.empty())
> + strTimeout="10000";
> + int numCon = lexical_cast<int>(strNumCon);
> + int timeout = lexical_cast<int>(strTimeout);
> + ServletContext& ctx = getServletConfig().getServletContext();
> + if(!ctx.hasAttribute("dbpool_attr")){
> + UuDbPool* pool = new UuDbPool(strDsn, strUid, strPwd, strDb, numCon, timeout);
> + ctx.setAttribute("dbpool_attr", setAttribute_t(new UuDbPool(*pool)));
> + }
> +}
> +
> +void UuServlet::destroy()
> +{
> +
> + ServletContext& ctx = getServletConfig().getServletContext();
> + if(ctx.hasAttribute("dbpool_attr")){
> + getDbPool()->shutdown();
> + ctx.removeAttribute("dbpool_attr");
> + delete getDbPool();
>
There is a really bad error here. Even two.
> + }
> +
> +}
> +
> +bool UuServlet::isUserLoggedIn (HttpServletRequest& req) const
> +{
> +
> + HttpSession* session = req.getSession(true);
> + if(!session) throw runtime_error("No session exist for this request");
>
Are you sure exception is warranted here? Why?
> + if(session->hasAttribute("user_attr")){
> + return true;
> + }
> + return false;
> +
> +}
> +
> +
> +/**
> + * Redirects browser to login page
> + * @param resp send redirect through this response object
> + */
> +void UuServlet::redirectToLogin (HttpServletResponse& resp)
> +{
> +
> + resp.sendRedirect("LoginServlet?new=Y");
> +
> +}
> +
> +
> +/**
> + * @return User
> + * @param req
> + */
> +
> +User UuServlet::getCurrentUser (HttpServletRequest& req)
> +{
> +
> + HttpSession* session = req.getSession(true);
> + if(!session) throw runtime_error("No session exist for this request");
>
Same as above. Why exception?
> + user_t user;
> + user = boost::static_pointer_cast<User>((session->getAttribute("user_attr")));
> + return *user;
> +
> +}
> +
> +/**
> + * @return UuDbPool*
> + */
> +
> +UuDbPool* UuServlet::getDbPool ()
> +{
> +
> + ServletContext& ctx = getServletConfig().getServletContext();
> + UuDbPool* pool = ctx.getAttribute<UuDbPool*>("dbpool_attr");
>
Use shared_ptr.
> + return pool;
> +
> +}
> +
> +
> +EXPORT_SERVLET(UuServlet);
> +
>
>
> Index: libui/RegisterServlet.h
> ===================================================================
> --- libui/RegisterServlet.h (revision 0)
> +++ libui/RegisterServlet.h (revision 0)
> @@ -0,0 +1,24 @@
> +#ifndef REGISTERSERVLET_H
> +#define REGISTERSERVLET_H
> +
> +#include <boost/lexical_cast.hpp>
> +#include "UuServlet.h"
> +
> +/**
> + * This servlet:
> + * Receives registration request.
> + * Accesses User model object.
> + * Stores results in request context.
> + * Determines the language to render page in based on user preferences.
> + * Forwards the request to the view object RegistrationView.csp.
> + */
> +
> +class RegisterServlet : public UuServlet
> +{
> +public:
> + void service(servlet::HttpServletRequest& request, servlet::HttpServletResponse &response);
> +
> +};
> +
> +#endif//REGISTERSERVLET_H
> +
>
>
> Index: libui/RegisterServlet.cpp
> ===================================================================
> --- libui/RegisterServlet.cpp (revision 0)
> +++ libui/RegisterServlet.cpp (revision 0)
> @@ -0,0 +1,66 @@
> +#include "RegisterServlet.h"
> +
> +using boost::lexical_cast;
> +using namespace std;
> +using namespace servlet;
> +
> +void RegisterServlet::service(HttpServletRequest& request, HttpServletResponse &response)
> +{
> +
> + bool isValidUser;
> + string csp = "RegistrationView.csp";
> + try{
> + isValidUser = isUserLoggedIn(request);
> + string newVisit = request.getParameter("new");
> + if(newVisit != "Y"){
> + string login = request.getParameter("login");
> + string password = request.getParameter("passwd");
> + string passwordc = request.getParameter("passwdc");
> + int language = boost::lexical_cast<int>(request.getParameter("lang"));
> + string firstName = request.getParameter("name1");
> + string lastName = request.getParameter("name2");
> + string street = request.getParameter("street");
> + string city = request.getParameter("city");
> + string state = request.getParameter("state");
> + string zip = request.getParameter("zipcode");
> + string country = request.getParameter("country");
> + string email = request.getParameter("email");
> + string phone = request.getParameter("phone");
> + int server = boost::lexical_cast<int>(request.getParameter("server"));
> + string message;
> + string defaultLogin="";
> + string defaultPassword="";
> + int defaultServer=1;
> + UuDbPool::Connection* con = getDbPool()->getConnection(defaultLogin, defaultPassword, defaultServer);
> + if(con == NULL)
> + throw std::logic_error("No DB connection exist for this request");
> + User* user = new User(*con, login);
> + bool isSuccess = user->setUserInfo(login, password, language, firstName, lastName, street, city, state, zip, country, email, phone);
> + delete user;
> + User* userLoggedIn = new User(*con, login);
>
Why are you creating user object twice?
> + if(isSuccess){
> + HttpSession* session = request.getSession(true);
> + if(!session)
> + throw logic_error("No session exist for this request");
> + if(session->hasAttribute("user_attr"))
> + session->removeAttribute("user_attr");
> + session->setAttribute("user_attr", setAttribute_t(new User(*userLoggedIn)));
> + session->setAttribute("server_attr", setAttribute_t(new int(server)));
> + message="Welcome new customer "+firstName+" "+lastName+"!";
> + csp = "HomeView.csp";
> + } else {
> + message = "Login name already exist";
> + }
>
1. Indentation please
2. Why do you need all these conditions above? If user is registering,
he should be logged out and session cleared before we even get here.
> + request.setAttribute("Message", setAttribute_t(new string(message)));
> + getDbPool()->releaseConnection(con);
> + }
> + }catch(const exception& ex) {
> + request.setAttribute("error", setAttribute_t(new string(ex.what())));
> + cerr<<__PRETTY_FUNCTION__<<": "<<ex.what()<<std::endl;
> + }
> + request.getRequestDispatcher(csp)->forward(request,response);
>
This won't work for us - remember class design had special method
for redirecting to view objects? Themes and all that..
> +
> +}
> +
> +EXPORT_SERVLET(RegisterServlet);
> +
>
>
> Index: libui/LoginServlet.h
> ===================================================================
> --- libui/LoginServlet.h (revision 0)
> +++ libui/LoginServlet.h (revision 0)
> @@ -0,0 +1,22 @@
> +#ifndef LOGINSERVLET_H
> +#define LOGINSERVLET_H
> +
> +#include "UuServlet.h"
> +
> +/**
> + * This servlet:
> + * Receives login request.
> + * Accesses User model object.
> + * Stores results in request context.
> + * Determines the language to render page in based on user preferences.
> + * Forwards the request to the view object LoginView.csp.
> + */
> +
> +class LoginServlet : public UuServlet
> +{
> +public:
> + void service(servlet::HttpServletRequest& request, servlet::HttpServletResponse &response);
> +};
> +
> +#endif//LOGINSERVLET_H
> +
>
>
> Index: libui/LoginServlet.cpp
> ===================================================================
> --- libui/LoginServlet.cpp (revision 0)
> +++ libui/LoginServlet.cpp (revision 0)
> @@ -0,0 +1,51 @@
> +#include "LoginServlet.h"
> +
> +using namespace std;
> +using namespace servlet;
> +
> +void LoginServlet::service(HttpServletRequest& request, HttpServletResponse &response)
> +{
> +
> + bool isValidUser;
> + string csp = "LoginView.csp";
> + try{
> + isValidUser = isUserLoggedIn(request);
> + string newVisit = request.getParameter("new");
> + if(newVisit != "Y"){
> + string login = request.getParameter("login");
> + string password = request.getParameter("CustPwd");
> + int server = boost::lexical_cast<int>(request.getParameter("server"));
>
Ugh.. I knew I was forgetting something. In this case I forgot to
write up on servers.. I am not sure where that idea came from,
and why it's in database design either.. One instance is bound to
_one_ server. Server type might be different - in that it may be
open server or closed server, the only real difference being in what
ACLs apply by default and what ACLs can be changed.
At some point in time I thought we might want to store different
courses in different databases - for load distribution purposes,
but it was never properly planned out, and now I think it was
stupid idea anyways.
> + string message;
> + UuDbPool::Connection* con = getDbPool()->getConnection(login, password, server);
> + if(con == NULL)
> + throw std::logic_error("No DB connection exist for this request");
> + User* user = new User(*con, login);
>
What if user passes emoty login?
> + bool isSuccess = user->isLoginPasswordValid(login, password);
>
This is useless. Database "login" procedure will fail already, if
password or login is
invalid.
> + if(isSuccess){
>
And this is considered bad style in most cases. Put function call into
conditional,
if you are not using result more then once.
> + HttpSession* session = request.getSession(true);
> + if(!session)
> + throw std::logic_error("No session exist for this request");
> + if(session->hasAttribute("user_attr"))
> + session->removeAttribute("user_attr");
> + session->setAttribute("user_attr", setAttribute_t(new User(*user)));
> + session->setAttribute("server_attr", setAttribute_t(new int(server)));
> + string firstname = getCurrentUser(request).getFirstName();
> + string lastname = getCurrentUser(request).getLastName();
> + message = "Welcome back, "+firstname+" "+lastname+"!";
> + csp = "HomeView.csp";
> + } else {
> + message = "Login/password combination is invalid";
> + }
> + request.setAttribute("Message", setAttribute_t(new string(message)));
> + getDbPool()->releaseConnection(con);
> + }
> + }catch(const exception& ex) {
> + request.setAttribute("error", setAttribute_t(new string(ex.what())));
> + cerr<<__PRETTY_FUNCTION__<<": "<<ex.what()<<std::endl;
> + }
> + request.getRequestDispatcher(csp)->forward(request,response);
> +
> +}
> +
> +EXPORT_SERVLET(LoginServlet);
> +
>
> Index: libui/LogoutServlet.h
> ===================================================================
> --- libui/LogoutServlet.h (revision 0)
> +++ libui/LogoutServlet.h (revision 0)
> @@ -0,0 +1,22 @@
> +#ifndef LOGOUTSERVLET_H
> +#define LOGOUTSERVLET_H
> +
> +#include "UuServlet.h"
> +
> +/**
> + * This servlet:
> + * Receives logout request.
> + * Cleans up HttpSession object.
> + * Stores results in request context.
> + * Determines the language to render page in based on user preferences.
> + * Forwards the request to the view object LoginView.csp.
> + */
> +
> +class LogoutServlet : public UuServlet
> +{
> +public:
> + void service(servlet::HttpServletRequest& request, servlet::HttpServletResponse &response);
> +};
> +
> +#endif//LOGOUTSERVLET_H
> +
>
>
> Index: libui/LogoutServlet.cpp
> ===================================================================
> --- libui/LogoutServlet.cpp (revision 0)
> +++ libui/LogoutServlet.cpp (revision 0)
> @@ -0,0 +1,30 @@
> +#include "LogoutServlet.h"
> +
> +using namespace std;
> +using namespace servlet;
> +
> +void LogoutServlet::service(HttpServletRequest& request, HttpServletResponse &response)
> +{
> +
> + string csp = "LoginView.csp";
> + string message;
> + try{
> + HttpSession* session = request.getSession(true); <-- why create session if you are going to destroy it right away?
> + if(!session)
> + throw std::logic_error("No session exist for this request"); <-- why is this an error?
> + session->invalidate();
> + HttpSession* sessionNew = request.getSession(true);
> + if(!sessionNew)
> + throw std::logic_error("No session exist for this request");
> + message="Logout successful";
> + request.setAttribute("Message", setAttribute_t(new string(message)));
> + }catch(const exception& ex) {
> + request.setAttribute("error", setAttribute_t(new string(ex.what())));
> + cerr<<__PRETTY_FUNCTION__<<": "<<ex.what()<<std::endl;
> + }
> + request.getRequestDispatcher(csp)->forward(request,response);
> +
> +}
> +
> +EXPORT_SERVLET(LogoutServlet);
> +
>
>
>
>
I'm skipping rest of servlets here, since most of the comments
above can be applied to them as well.
> Index: libdm/User.h
> ===================================================================
> --- libdm/User.h (revision 0)
> +++ libdm/User.h (revision 0)
> @@ -0,0 +1,49 @@
> +#ifndef USER_H
> +#define USER_H
> +
> +#include <string>
> +#include <boost/lexical_cast.hpp>
> +#include <map>
> +#include <vector>
> +
> +#include "UuDbPool.h"
> +#include "SessionInfo.h"
> +
> +/**
> + *This class accesses and alters person_list, city_list tables.
> + */
> +
> +class User
> +{
> +public:
> + User(UuDbPool::Connection&, std::string);
> + virtual ~User();
> +
> + private:
> + UuDbPool::Connection& m_handle;
> + std::string m_login;
> + std::string m_password;
> + std::string m_firstname;
> + std::string m_lastname;
> +
> + public:
> + virtual std::string getLogin();
> + virtual std::string getPassword();
> + virtual std::string getFirstName();
> + virtual std::string getLastName();
> +
> + /**
> + * Performs user authentication
> + */
> +
> + bool isLoginPasswordValid(std::string login, std::string password);
> +
> + /**
> + * Sets all user data during a registration process. Calls stored procedure "person_create" to insert user data into "person_list" table. If insert is successful, returns true. Otherwise returns false.
> + */
> +
> + virtual bool setUserInfo ( std::string login, std::string password, int language, std::string firstName, std::string lastName, std::string street, std::string city, std::string state, std::string zip, std::string country, std::string email, std::string phone );
> +
> +};
> +
> +#endif // USER_H
>
>
> Index: libdm/User.cpp
> ===================================================================
> --- libdm/User.cpp (revision 0)
> +++ libdm/User.cpp (revision 0)
> @@ -0,0 +1,111 @@
> +#include "User.h"
> +
> +using namespace std;
> +using namespace sptk;
> +using namespace boost;
> +
> +User::User(UuDbPool::Connection& handle, string login)
> + : m_handle(handle)
> + , m_login(login)
> +{
> +
> + try {
> + CQuery qrySelect(&m_handle,"select pl_login, pl_password, pl_first_name, pl_last_name from person_list where pl_login=:login");
>
This will break with latest database schema.
> + qrySelect.param("login") = m_login;
> + qrySelect.open();
> + CField& passField = qrySelect["pl_password"];
> + CField& fnameField = qrySelect["pl_first_name"];
> + CField& lnameField = qrySelect["pl_last_name"];
> + while ( ! qrySelect.eof() ) {
> + m_password = passField.asString();
> + m_firstname = fnameField.asString();
> + m_lastname = lnameField.asString();
> + qrySelect.fetch();
> + }
> + qrySelect.close();
> + }
> + catch (exception& e) {
> + cout<<"\nError: " <<e.what();
> + }
> +
> +}
> +User::~User ( )
> +{}
> +
> +
> +string User::getLogin()
> +{
> +
> + return m_login;
> +
> +}
> +
> +string User::getPassword()
> +{
> +
> + return m_password;
> +
> +}
> +
> +string User::getFirstName()
> +{
> +
> + return m_firstname;
> +
> +}
> +
> +string User::getLastName()
> +{
> +
> + return m_lastname;
> +
> +}
> +
> +/**
> + * Performs authentication.
> + * @return bool
> + */
> +
> +bool User::isLoginPasswordValid(string login, string password)
>
This function isn't needed at all.
> +{
> +
> + bool isValid = false;
> + try {
> + CQuery qrySelect(&m_handle,"select pl_id from person_list where pl_login=:login and pl_password=:password");
> + qrySelect.param("login") = login;
> + qrySelect.param("password") = password;
> + qrySelect.open();
> + while ( ! qrySelect.eof() ) {
> + isValid = true;
> + qrySelect.fetch();
> + }
> + qrySelect.close();
> + }
> + catch (exception& e) {
> + cout<<"\nError: " <<e.what();
> + return false;
> + }
> + return isValid;
> +
> +}
> +
> +
> +/**
> + * Sets all user data during a registration process. Calls stored procedure "person_create" to insert user data into "person_list" table. If insert is successful, returns true. Otherwise returns false.
> + */
> +
> +bool User::setUserInfo(string login, string password, int language, string firstName, string lastName, string street, string city, string state, string zip, string country, string email, string phone )
> +{
> +
> + try {
> + CQuery qryInsert(&m_handle,"select person_create('"+firstName+"','"+lastName+"','"+street+"','"+city+"','"+state+"','"+zip+"','"+country+"','"+email+"','"+phone+"','"+login+"','"+password+"')");
>
Never ever ever ever use concatenation to form SQL query.
It's security bug. It's inefficient. It's just wrong. Use bound
variables.
> + qryInsert.exec();
> + }
> + catch (exception& e) {
> + cout<<"\nError: " <<e.what();
> + return false;
> + }
> + return true;
> +
> +}
> +
>
>
> Index: libdm/UuDbPool.h
> ===================================================================
> --- libdm/UuDbPool.h (revision 0)
> +++ libdm/UuDbPool.h (revision 0)
> @@ -0,0 +1,41 @@
> +#include <sptk3/CODBCDatabase.h>
> +#include <sptk3/CException.h>
> +#include <sptk3/CQuery.h>
> +#include <sptk3/CThread.h>
> +#include <sptk3/CWaiter.h>
> +
> +#include <list>
> +#include <boost/lexical_cast.hpp>
> +
> +class UuDbPool
> +{
> +public:
> + class Connection: public sptk::CODBCDatabase
> + {
> + public:
> + Connection(UuDbPool* pool)
> + : sptk::CODBCDatabase(pool->m_conString)
> + {}
> + };
>
What good does this class do you?
> +private:
> + typedef std::list<Connection*> connectionlist_t;
> +private:
> + connectionlist_t m_connections;
> + std::string m_conString;
> + int m_maxCon;
> + int m_checkedOut;
>
I am not sure you need this variable at all.
> + int m_timeout;
> + sptk::CWaiter m_lock;
> +public:
> + UuDbPool(std::string dsn, std::string dbuser, std::string dbpassword, std::string db, int maxCon, int timeout);
> + ~UuDbPool();
> +public:
> + int& getMaxCon();
> + int& getTimeout();
>
Why do these functions return reference?
> + Connection* getConnection(std::string login, std::string password, int server);
> + void releaseConnection(Connection* con);
> + void shutdown();
> +
> +};
> +
> +
>
>
> Index: libdm/UuDbPool.cpp
> ===================================================================
> --- libdm/UuDbPool.cpp (revision 0)
> +++ libdm/UuDbPool.cpp (revision 0)
> @@ -0,0 +1,129 @@
> +#include "UuDbPool.h"
> +
> +using namespace std;
> +using namespace sptk;
> +using namespace boost;
> +
> +UuDbPool::UuDbPool(string dsn, string dbuser, string dbpassword, string db, int maxCon, int timeout)
> + : m_conString("DSN="+dsn+";UID="+dbuser+";PWD="+dbpassword+";DATABASE="+db)
> + , m_maxCon(maxCon)
> + , m_checkedOut(0)
> + , m_timeout(timeout)
> +
> +{
> +
> + m_lock.lock();
> + getMaxCon(); <<---------------------???
> + getTimeout(); <<--------------------???
> + for(int i=0;i<maxCon;i++){
> + Connection* con=new Connection(this);
> + m_connections.push_front(con);
> + con->open();
> + }
> + m_lock.unlock();
> +
> +}
> +
> +UuDbPool::~UuDbPool()
> +{
> +
> + shutdown();
> +
> +}
> +
> +
> +int& UuDbPool::getMaxCon()
> +{
> +
> + static int maxCon(m_maxCon);
>
What exactly are you trying to do here?
> + return maxCon;
> +
> +}
> +
> +
> +int& UuDbPool::getTimeout()
> +{
> +
> + static int timeout(m_timeout);
> + return timeout;
> +
> +}
> +
> +UuDbPool::Connection* UuDbPool::getConnection(string login, string password, int server)
> +{
> +
> + m_lock.lock();
> + while(m_checkedOut >= getMaxCon()){
> + m_lock.waitForSignal(getTimeout());
>
Why is it inside of while loop?
> + }
> + Connection* con = 0;
> + if(m_connections.size() > 0){
>
std::list::size() is O(N) operation. comparing begin() and end() is far
more efficient.
> + connectionlist_t::iterator iter = m_connections.begin();
> + con = *iter;
> + if(!con->active()){
> + con->open();
> + }
> + m_connections.erase(iter);
> + }
> + if(con != NULL){
> + m_checkedOut++;
> + try{
> + CQuery loginQuery(con,"select login(:login,:password,:server)");
> + loginQuery.param("login") = login;
> + loginQuery.param("password") = password;
> + loginQuery.param("server") = server;
> + loginQuery.exec();
> + }
> + catch (exception& e) {
> + cout<<"\nError: " <<e.what();
> + con = NULL;
> + }
> + }
> + //m_lock.broadcastSignalNoLock();
>
Why are you trying to broadcast the signal in the first place?
> + m_lock.unlock();
> + return con;
> +
> +}
> +
> +
> +void UuDbPool::releaseConnection(Connection* con)
> +{
> +
> + m_lock.lock();
> + if(con != NULL){
> + if(con->active()){
> + m_connections.push_back(con);
> + m_checkedOut--;
> + try {
> + CQuery logoutQuery(con,"select logout()");
> + logoutQuery.exec();
> + }
> + catch (exception& e) {
> + cout<<"\nError: " <<e.what();
> + }
>
I'd log out before pushing it back into queue.
> + }
> + }
> + // m_lock.broadcastSignalNoLock();
> + m_lock.unlock();
> +
> +}
> +
> +
> +void UuDbPool::shutdown()
>
Race condition here. And you forgot to wait for
connections retrieved from the pool to be returned.
> +{
> +
> + if(m_connections.size() > 0){
> + connectionlist_t::iterator it;
> + for(it=m_connections.begin();
> + it!=m_connections.end(); it++)
> + {
> + (*it)->close();
> + }
> + m_connections.clear(); <<<<<<<<<<<<--------- How do you think, what does this function do?
> + m_connections.erase(it);
> + }
> + delete this;
> +
> +}
> +
> +
>
> Index: libui/Makefile.adon
> ===================================================================
> --- libui/Makefile.adon (revision 82)
> +++ libui/Makefile.adon (working copy)
> @@ -0,0 +1,9 @@
> +## libui Makefile
> +
> +noinst_LTLIBRARIES:=libuu
> +noinst_HEADERS:=UuServlet.h HomeServlet.h RegisterServlet.h LoginServlet.h LogoutServlet.h AccountServlet.h
> +libuu_SOURCES:=UuServlet.cpp HomeServlet.cpp RegisterServlet.cpp LoginServlet.cpp LogoutServlet.cpp AccountServlet.cpp
> +libuu_LDFLAGS:=-lspdb3 -lsputil3 -luu -ldm
> +libuu_DEPS:=libuu
>
Why are you making libuu depend on libuu?
> +
> +ADON_SUBDIRS:=
>
No need for empty subdirs var.
> Index: engine.xml
> ===================================================================
> --- engine.xml (revision 0)
> +++ engine.xml (revision 0)
> @@ -0,0 +1,21 @@
> +<?xml version="1.0"?>
> +<listener protocol="unix" path="/tmp/cppserv.sock"/>
> +<app name="">
> + <servlet name="UuServlet" dso="./debug/libui/libuu.so">
>
Can't do this. UuServlet is not for instantiation.
> + <parameter name="DSN" value="PostgreSQL"/>
> + <parameter name="UID" value="sergey"/>
> + <parameter name="PWD" value=""/>
> + <parameter name="DB" value="uu"/>
> + <parameter name="num_connections" value="5"/>
> + <parameter name="timeout" value="10000"/>
> + </servlet>
> + <servlet name="HomeServlet" dso="./debug/libui/libuu.so"/>
> + <servlet name="RegisterServlet" dso="./debug/libui/libuu.so"/>
> + <servlet name="LoginServlet" dso="./debug/libui/libuu.so"/>
> + <servlet name="LogoutServlet" dso="./debug/libui/libuu.so"/>
> + <servlet name="AccountServlet" dso="./debug/libui/libuu.so"/>
> + <csp name="themes/default/HomeView.csp" path="HomeView.csp" dso="./debug/themes/default/HomeView.so" hidden="true"/>
> + <csp name="themes/default/RegistrationView.csp" path="RegistrationView.csp" dso="./debug/themes/default/RegistrationView.so" hidden="true"/>
> + <csp name="themes/default/LoginView.csp" path="LoginView.csp" dso="./debug/themes/default/LoginView.so" hidden="true"/>
> + <csp name="themes/default/AccountView.csp" path="AccountView.csp" dso="./debug/themes/default/AccountView.so" hidden="true"/>
> +</app>
--
Ilya A. Volynets-Evenbakh
Total Knowledge. CTO
http://www.total-knowledge.com