Ensure sockets are closed when sessions are closed #1910

mohandev2 · 2016-03-01T02:45:26Z

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

Reported by: mtomlinson

Original Ticket: openhpi/bugs/1910

The text was updated successfully, but these errors were encountered:

mohandev2 · 2016-03-01T02:47:50Z

Oops, formatting makes this patch a bit messy. I have added it as an attachment.
0001-Ensure-sockets-are-closed-when-sessions-are-closed.patch.txt

Original comment by: mtomlinson

mohandev2 · 2016-04-06T15:14:38Z

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

Original comment by: erpraveenk

mohandev2 · 2016-04-10T04:36:34Z

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:

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

[bugs:#1910] https://sourceforge.net/p/openhpi/bugs/1910/ Ensure
sockets are closed when sessions are closed

Status: open
3.7.0: 3.7.0
Created: Tue Mar 01, 2016 02:45 AM UTC by Mark Tomlinson
Last Updated: Tue Mar 01, 2016 02:47 AM UTC
Owner: nobody

From cf9cc9b59586bcdb410d03c6b26408eb4eb75414 Mon Sep 17 00:00:00 2001
From: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz
mailto: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
<mailto: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
Sent from sourceforge.net because you indicated interest in
https://sourceforge.net/p/openhpi/bugs/1910/

To unsubscribe from further messages, please visit
https://sourceforge.net/auth/subscriptions/

Original comment by: mtomlinson

mohandev2 · 2016-04-14T23:00:32Z

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
1910_with_prints.patch.txt

Original comment by: mohandev2

mohandev2 · 2016-05-26T00:29:10Z

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

Original comment by: mohandev2

mohandev2 · 2017-06-05T20:13:34Z

labels: --> OpenHPI Daemon Client
3.7.0: 3.7.0 --> 3.8.0

Original comment by: mohandev2

mohandev2 · 2018-02-27T23:52:02Z

3.7.0: 3.8.0 --> Future

Original comment by: mohandev2

mohandev2 added OpenHPI Daemon Client sourceforge auto-migrated Future labels Mar 28, 2018

mohandev2 assigned openhpi2 Mar 28, 2018

mohandev2 added this to the Future milestone Apr 28, 2018

mohandev2 removed the Future label May 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure sockets are closed when sessions are closed #1910

Ensure sockets are closed when sessions are closed #1910

mohandev2 commented Mar 1, 2016

mohandev2 commented Mar 1, 2016 •

edited

Loading

mohandev2 commented Apr 6, 2016

mohandev2 commented Apr 10, 2016

mohandev2 commented Apr 14, 2016 •

edited

Loading

mohandev2 commented May 26, 2016

mohandev2 commented Jun 5, 2017

mohandev2 commented Feb 27, 2018

Ensure sockets are closed when sessions are closed #1910

Ensure sockets are closed when sessions are closed #1910

Comments

mohandev2 commented Mar 1, 2016

Signed-off-by: Mark Tomlinson mark.tomlinson@alliedtelesis.co.nz

mohandev2 commented Mar 1, 2016 • edited Loading

mohandev2 commented Apr 6, 2016

System Details:

Erros while compilation:

mohandev2 commented Apr 10, 2016

mohandev2 commented Apr 14, 2016 • edited Loading

mohandev2 commented May 26, 2016

mohandev2 commented Jun 5, 2017

mohandev2 commented Feb 27, 2018

mohandev2 commented Mar 1, 2016 •

edited

Loading

mohandev2 commented Apr 14, 2016 •

edited

Loading