#1910 Ensure sockets are closed when sessions are closed
From cf9cc9b59586bcdb410d03c6b26408eb4eb75414 Mon Sep 17 00:00:00 2001
From: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz
Date: Mon, 29 Feb 2016 13:21:57 +1300
Subject: [PATCH] Ensure sockets are closed when sessions are closed
The cSession class had a per-thread variable m_sockets. This allowed
different threads to share the same session, but each would have a
different socket to communicate with the client. This was implemented
using GStaticPrivate, but as this had been deprecated, changed to
GPrivate. The documentation for GPrivate states: "It is not possible to
destroy a GPrivate after it has been used. As such, it is only ever
acceptable to use GPrivate in static scope, and even then sparingly so."
This is not true for the m_sockets variable.
To achieve the same thing, a hash table is used to allow a different
socket per thread. When the cSession class is deleted, all sockets are
closed as the hash table is freed up.
Signed-off-by: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz
baselib/session.cpp | 34 ++++++++++++----------------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/baselib/session.cpp b/baselib/session.cpp
index c5edfc8..ca9c2d7 100644
--- a/baselib/session.cpp
+++ b/baselib/session.cpp
@@ -103,11 +103,7 @@ private:
SaHpiDomainIdT m_did;
SaHpiSessionIdT m_sid;
SaHpiSessionIdT m_remote_sid;
-#if GLIB_CHECK_VERSION (2, 32, 0)
- GPrivate m_sockets;
-#else
- GStaticPrivate m_sockets;
-#endif
+ GHashTable * m_sockets;
};
@@ -117,16 +113,12 @@ cSession::cSession()
m_sid( 0 ),
m_remote_sid( 0 )
{
- #if GLIB_CHECK_VERSION (2, 32, 0)
- m_sockets = G_PRIVATE_INIT (g_free);
- #else
- wrap_g_static_private_init( &m_sockets );
- #endif
+ m_sockets = g_hash_table_new_full( g_direct_hash, g_direct_equal, 0, DeleteSock );
}
cSession::~cSession()
{
- wrap_g_static_private_free( &m_sockets );
+ g_hash_table_destroy( m_sockets );
}
SaErrorT cSession::GetEntityRoot( SaHpiEntityPathT& entity_root ) const
@@ -213,11 +205,9 @@ SaErrorT cSession::DoRpc( uint32_t id,
}
}
-
if GLIB_CHECK_VERSION (2, 32, 0)
- wrap_g_static_private_set( &m_sockets, 0);// close socket
-
else
- wrap_g_static_private_set( &m_sockets, 0, 0 ); // close socket
-
endif
- ohc_lock();
- g_hash_table_remove( m_sockets, (void *)pthread_self() ); // close socket
- ohc_unlock();
g_usleep( NEXT_RPC_ATTEMPT_TIMEOUT );
}
if ( !rc ) {
@@ -240,7 +230,9 @@ SaErrorT cSession::DoRpc( uint32_t id,
SaErrorT cSession::GetSock( cClientStreamSock * & sock )
{
- gpointer ptr = wrap_g_static_private_get( &m_sockets );
+ ohc_lock();
+ gpointer ptr = g_hash_table_lookup( m_sockets, (void )pthread_self() );
+ ohc_unlock();
if ( ptr ) {
sock = reinterpret_cast<cClientStreamSock *="">(ptr);
} else {
@@ -266,11 +258,9 @@ SaErrorT cSession::GetSock( cClientStreamSock * & sock )
/ keepalive_intvl / 1,
/ keepalive_probes */ 3 );
-
if GLIB_CHECK_VERSION (2, 32, 0)
- wrap_g_static_private_set( &m_sockets, sock );
-
else
- wrap_g_static_private_set( &m_sockets, sock, DeleteSock );
-
endif
- ohc_lock();
- g_hash_table_insert ( m_sockets, (void *)pthread_self(), sock );
-
ohc_unlock();
}return SA_OK;
2.7.2
Oops, formatting makes this patch a bit messy. I have added it as an attachment.
Hi,
I appiled the attached patch but I am getting below errors while compilation. Do we have any updated patch.
System Details:
LSB Version: :core-4.0-amd64:core-4.0-noarch:graphics-4.0-amd64:graphics-4.0-noarch:printing-4.0-amd64:printing-4.0-noarch
Distributor ID: RedHatEnterpriseServer
Description: Red Hat Enterprise Linux Server release 6.1 (Santiago)
Release: 6.1
Codename: Santiago
Erros while compilation:
session.cpp: In member function ‘SaErrorT cSession::DoRpc(uint32_t, ClientRpcParams&, ClientRpcParams&)’:
session.cpp:209: error: ‘pthread_self’ was not declared in this scope
session.cpp: In member function ‘SaErrorT cSession::GetSock(cClientStreamSock*&)’:
session.cpp:234: error: ‘pthread_self’ was not declared in this scope
make[2]: [session.lo] Error 1
make[2]: Leaving directory
/root/praveen/1910/openhpi_april_06_2016/baselib' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory/root/praveen/1910/openhpi_april_06_2016'make: [all] Error 2
Thanks!
Praveen
I just downloaded the latest trunk and applied the patch as-is, and it
worked fine for me. I am using Ubuntu, so I guess there's some
difference somewhere between our systems. Even though I can't reproduce
the issue, the only thing that appears to be wrong is not having a
prototype for pthread_self(). Please try adding the following to
session.cpp:
@@ -20,6 +20,7 @@
#include <string.h>
#include <glib.h>
+#include <pthread.h>
#include <oh_error.h>
This compiles fine for me, but then again, it did without this change.
On 07/04/16 03:14, Praveen wrote:
Related
Bugs: #1910
Mark,
Thanks for the patch. I applied it with addional printf/CRIT statements and ran the tests using hpitop and hpi_shell. By and large it works well. The hpitop closed the sock properly. The hpi_shell has two threads and somehow the socks are not closed on that. Still looking into it. The modified patch is attached.
Mohan
Mark,
The only client library that uses multiple threads wich sock connections is hpi_shell. Other single threaded applications are closing the sockets properly. At present hpi_shell does not. Even with your patch it is not closing the sockets at the end of the session. Could you please take a look at hpi_shell with your patch?
Thanks
Mohan