thread safe usage of multiple phones

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

thread safe usage of multiple phones

Peter Koch-12
Peter

Dear Gnokii experts,

I'm using libgnokii for a very long time to sent certain log messages via SMS
to our administrators. A multithreaded application receives log messages
from different sources and stores them into our database. Urgent messages
will also be sent via SMS to our administrators.

Unfortunately the single Nokia 6310 phone (connected via serial cable)
has become a perfomance bottleneck and we are planning to use 5 Nokia 6230i
(connected via USB) instead.

Right now the daemon has one thread that sends out SMS and I would
like to start 4 more threads that will do the SMS-sending in parallel.

I'm afraight not all libgnokii routines are thread-safe. gn_cfg_info and
gn_gsm_info are two global variables that prevent concurrent usage of
at least gn_lib_phoneprofile_load_from_file(). But that's not a problem
since initialization of the phones can be done non-concurrent.

But what about gn_sms_send()? The whole idea is to call this routine
concurrently with 5 phones.

Can I call gn_sms_send(data1, state1) and gn_sms_send(data2, state2)
in two threads?

Here's a short summary of what I'm planning to do:

In main thread:
----------------------------------------------------------------------
for(i=0; i<max; ++i){
  gn_lib_phoneprofile_load_from_file("gnokiirc", configname[i], &state[i]);
  gn_lib_phone_open(state[i]);
}
for(i=0; i<max; ++i){
  start_separate_thread(&state[i]);
}

In separate threads:
----------------------------------------------------------------------
for(;;){
  wait_for_sms();
  get_sms_from_queue(&data);
  gn_sms_send(data, &state);
}

Any ideas or suggestions?

Kind regards

Peter

_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users
Reply | Threaded
Open this post in threaded view
|

Re: thread safe usage of multiple phones

Pawel Kot
Hi Peter,

On Sat, Apr 15, 2017 at 12:45 PM, Peter Koch <[hidden email]> wrote:
> I'm using libgnokii for a very long time to sent certain log messages via SMS
> to our administrators. A multithreaded application receives log messages
> from different sources and stores them into our database. Urgent messages
> will also be sent via SMS to our administrators.

Well, for me personally it is quite nice surprise that in the age of
smartphones and API to everything gnokii is still being used :)

> Unfortunately the single Nokia 6310 phone (connected via serial cable)
> has become a perfomance bottleneck and we are planning to use 5 Nokia 6230i
> (connected via USB) instead.
>
> Right now the daemon has one thread that sends out SMS and I would
> like to start 4 more threads that will do the SMS-sending in parallel.

I do not know how it would behave right now but years ago USB link was
much less reliable than serial one. To translate it to actions: it
required restarting gnokii or even phone sometimes.

> I'm afraight not all libgnokii routines are thread-safe. gn_cfg_info and
> gn_gsm_info are two global variables that prevent concurrent usage of
> at least gn_lib_phoneprofile_load_from_file(). But that's not a problem
> since initialization of the phones can be done non-concurrent.

I think these are deprecated so that should be no problem - there is a
way to deal without them.

> But what about gn_sms_send()? The whole idea is to call this routine
> concurrently with 5 phones.

I have looked quickly into the code. In gsm-sms.c just
sms_timestamp_print() seems to be thread non-safe (just in DEBUG mode)

> Can I call gn_sms_send(data1, state1) and gn_sms_send(data2, state2)
> in two threads?

If you do the preparation by yourself, I think that would work fine.
But it would be the best just to try it out. Looks like 1-day
excersise.

> Here's a short summary of what I'm planning to do:
>
> In main thread:
> ----------------------------------------------------------------------
> for(i=0; i<max; ++i){
>   gn_lib_phoneprofile_load_from_file("gnokiirc", configname[i], &state[i]);
>   gn_lib_phone_open(state[i]);
> }
> for(i=0; i<max; ++i){
>   start_separate_thread(&state[i]);
> }
>
> In separate threads:
> ----------------------------------------------------------------------
> for(;;){
>   wait_for_sms();

To you know how to dinstinguish between phones? One queue or multiple queues?

>   get_sms_from_queue(&data);
>   gn_sms_send(data, &state);
> }
>
> Any ideas or suggestions?

The easiest way (that is using gnokii in non-modified form) would be
to run multiple gnokii (or smsd) instances. And then feed them with
round-robin or so.

take care,
Pawel
--
http://www.gnokii.org/

_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users
Reply | Threaded
Open this post in threaded view
|

Re: thread safe usage of multiple phones

Peter Koch-12
Dear Pawel:

First let me congratulate you for this wonderful piece of software.
We are using it for almost 10 years. It's rock stable and is sending
notifications to our adminsitrators in situations where emails just
dont' work (or mail to SMS gateways).

Well, for me personally it is quite nice surprise that in the age of
smartphones and API to everything gnokii is still being used :)

We were thinking about other notification mechanisms but so far
a mobile phone that is directly connected to our logserver via serial
cable is the most reliable and secure way to inform us about problems.
 
I do not know how it would behave right now but years ago USB link was
much less reliable than serial one. To translate it to actions: it
required restarting gnokii or even phone sometimes.

We started in 2008 with gnokii-0.6.23, a Nokia 6310 phone and a DLR3P
serial cable. And once in a while we had those instabilities that you
mentioned. Phone and software had to be restarted. Therefore we
changed our software such that every administrator got a single
message every day at 12:00. If this heartbeat-SMS was missing we knew
that there was a gnokii-problem and someone had to restart the phone.

When we changed to gnokii-0.6.30 and connection=AT all these
problems disappeared.

Also, I'm closing the gnokii-connection whenever the messge queue
becomes empty. With a serial connection reopening the connection
is slow but this improved stability very much. I'm hoping that reopening
a connection with USB-connected phones will be a lot faster.

My hope is that multiple phones will add another level of redundancy.
If one phone hangs then messages will be resent by one of the other
phones. Therefore I will keep the Nokia 6310, so if the USB-bus hangs
and all of the USB-connected phones cannot sent SMS anymore at
least the serial connection will work.

So far a Nokia 6230i with CA-53 cable and connection=dku2libusb
seems to work very well with gnokii-0.6.31.

I have looked quickly into the code. In gsm-sms.c just
sms_timestamp_print() seems to be thread non-safe (just in DEBUG mode)

> Can I call gn_sms_send(data1, state1) and gn_sms_send(data2, state2)
> in two threads?

If you do the preparation by yourself, I think that would work fine.
But it would be the best just to try it out. Looks like 1-day
excersise.

I'll do that within the next days

Do you know how to dinstinguish between phones? One queue or multiple queues?

>   get_sms_from_queue(&data);
>   gn_sms_send(data, &state);
> }
>
> Any ideas or suggestions?

The easiest way (that is using gnokii in non-modified form) would be
to run multiple gnokii (or smsd) instances. And then feed them with
round-robin or so.

With multiple gnokii or smsd instances it would be hard to detect
problems in one instance and resent messages that failed on one
phone with a different phone.

The gnokii-part within my own daemon ist almost completely stolen
from your source. There's already a queue that is filled from different
threads that collect messages from different sources. There's one thread
that is feeding all these messages into our Oracle Database. And
there is another thread that is sending urgent messages out to our
administrators. Adding more SMS-threads should be relatively easy.
And if one thread has problems it should put its message back into
the queue and stop itself.

I'm sure that will work for another 10 years :-)

Kind regards

Peter Koch

_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users
Reply | Threaded
Open this post in threaded view
|

Re: thread safe usage of multiple phones

Jan Derfinak
Peter Koch wrote:

Hello.

>
> With multiple gnokii or smsd instances it would be hard to detect
> problems in one instance and resent messages that failed on one
> phone with a different phone.

It is hard to me understand statement above. You can monitor all
instances trough one database and if you see an error in one instance,
you can move messages to other instances by simple change in phone
identification column.

Jan




_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users
Reply | Threaded
Open this post in threaded view
|

Re: thread safe usage of multiple phones

Peter Koch-12
In reply to this post by Pawel Kot
Dear Pawel

we are using multiple nokia phones now with our multi-threaded
sms-daemon. Works well so far

I had to fix one problem and I did it the quick+dirty way.

Have a look at routine verify_max_message_len() in
common/links/fbus-phonet.c, starting at line 64

static int verify_max_message_len(int len, char **message_buffer)
{
        static int max_message_len = 0;

        if (len > max_message_len) {
                dprintf("overrun: %d %d\n", len, max_message_len);
                *message_buffer = realloc(*message_buffer, len + 1);
                max_message_len = len + 1;
        }
        if (*message_buffer)
                return max_message_len;
        else
                return 0;
}

The static declaration of max_message_len causes problems if
multiple thread are calling this routine. Even worse. When opening
a phone max_message_len must be 0. Otherwise gn_lib_phone_open()
will fail.

This makes the following code fail on the second invocation of
gn_lib_phone_open(). max_message_len stays non-zero after
the first call of verify_max_message_len() and therefor no memory
will be allocated in the second call. *message_buffer will be NULL
and gn_lib_phone_open() fails.

Of course this only happens when the phonet-driver is used, so my
serial nokia 6310 works fine.

main(){
  struct gn_statemachine *state;
  gn_error err;

  err=gn_lib_phoneprofile_load_from_file(GNOKIIRC, NULL, &state);
  printf("load=%d, %s\n", err, gn_error_print(err));
  err=gn_lib_phone_open(state);
  printf("open=%d, %s\n", err, gn_error_print(err));
  err=gn_lib_phone_close(state);
  printf("close=%d, %s\n", err, gn_error_print(err));
  err=gn_lib_phone_open(state); // err will be 9 with nokia 6230i and dku2libusb
  printf("open=%d, %s\n", err, gn_error_print(err));
}

Here's my dirty fix for routine verify_max_message_len()

static int verify_max_message_len(int len, char **message_buffer)
{
        static int max_message_len = 0;

        if(len<PHONET_FRAME_MAX_LENGTH) len=PHONET_FRAME_MAX_LENGTH;
        if(len>PHONET_FRAME_MAX_LENGTH || !*message_buffer){
                dprintf("reallocating message_buffer to %d bytes\n", len+1);
                *message_buffer = realloc(*message_buffer, len+1);
        }
        return *message_buffer ? len+1 : 0;

        if (len > max_message_len) {
                dprintf("overrun: %d %d\n", len, max_message_len);
                *message_buffer = realloc(*message_buffer, len + 1);
                max_message_len = len + 1;
        }
        if (*message_buffer)
                return max_message_len;
        else
                return 0;
}

Peter

_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users
Reply | Threaded
Open this post in threaded view
|

Re: thread safe usage of multiple phones

Pawel Kot
Hi Peter,

On Sun, May 14, 2017 at 9:31 AM, Peter Koch <[hidden email]> wrote:
> Have a look at routine verify_max_message_len() in
> common/links/fbus-phonet.c, starting at line 64

Yeah, that makes sense. I have reviewed just statemachine and sms
code. And they seem thread safe. I have forgotten the link code. It
indeed assumes communication with one device.

> Of course this only happens when the phonet-driver is used, so my
> serial nokia 6310 works fine.

I will have a closer look.

> static int verify_max_message_len(int len, char **message_buffer)
> {
>         static int max_message_len = 0;
>
>         if(len<PHONET_FRAME_MAX_LENGTH) len=PHONET_FRAME_MAX_LENGTH;
>         if(len>PHONET_FRAME_MAX_LENGTH || !*message_buffer){
>                 dprintf("reallocating message_buffer to %d bytes\n", len+1);
>                 *message_buffer = realloc(*message_buffer, len+1);
>         }
>         return *message_buffer ? len+1 : 0;

Quite frankly at the moment I'm not sure why would it work.

>         if (len > max_message_len) {
>                 dprintf("overrun: %d %d\n", len, max_message_len);
>                 *message_buffer = realloc(*message_buffer, len + 1);
>                 max_message_len = len + 1;
>         }
>         if (*message_buffer)
>                 return max_message_len;
>         else
>                 return 0;
> }

Take care,
Pawel

_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users
Reply | Threaded
Open this post in threaded view
|

Re: thread safe usage of multiple phones

Pawel Kot
Well, it looks like the solution I have implemented was quite dumb.
This static variable was really not necessary (however it made
solution simpler). Can you please try the attached patch and say
whether it works for you? It is against git version.

take care,
Pawel

On Sun, May 14, 2017 at 1:29 PM, Pawel Kot <[hidden email]> wrote:

> Hi Peter,
>
> On Sun, May 14, 2017 at 9:31 AM, Peter Koch <[hidden email]> wrote:
>> Have a look at routine verify_max_message_len() in
>> common/links/fbus-phonet.c, starting at line 64
>
> Yeah, that makes sense. I have reviewed just statemachine and sms
> code. And they seem thread safe. I have forgotten the link code. It
> indeed assumes communication with one device.
>
>> Of course this only happens when the phonet-driver is used, so my
>> serial nokia 6310 works fine.
>
> I will have a closer look.
>
>> static int verify_max_message_len(int len, char **message_buffer)
>> {
>>         static int max_message_len = 0;
>>
>>         if(len<PHONET_FRAME_MAX_LENGTH) len=PHONET_FRAME_MAX_LENGTH;
>>         if(len>PHONET_FRAME_MAX_LENGTH || !*message_buffer){
>>                 dprintf("reallocating message_buffer to %d bytes\n", len+1);
>>                 *message_buffer = realloc(*message_buffer, len+1);
>>         }
>>         return *message_buffer ? len+1 : 0;
>
> Quite frankly at the moment I'm not sure why would it work.
>
>>         if (len > max_message_len) {
>>                 dprintf("overrun: %d %d\n", len, max_message_len);
>>                 *message_buffer = realloc(*message_buffer, len + 1);
>>                 max_message_len = len + 1;
>>         }
>>         if (*message_buffer)
>>                 return max_message_len;
>>         else
>>                 return 0;
>> }
>
> Take care,
> Pawel


--
Pawel Kot

_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users

fbusphonet.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: thread safe usage of multiple phones

Ladislav Michl
In reply to this post by Peter Koch-12
Hi Peter,

(and sorry for such a late reply)

On Sun, May 14, 2017 at 09:31:55AM +0200, Peter Koch wrote:

> Dear Pawel
>
> we are using multiple nokia phones now with our multi-threaded
> sms-daemon. Works well so far
>
> I had to fix one problem and I did it the quick+dirty way.
>
> Have a look at routine verify_max_message_len() in
> common/links/fbus-phonet.c, starting at line 64
>
> static int verify_max_message_len(int len, char **message_buffer)
> {
>         static int max_message_len = 0;
>
>         if (len > max_message_len) {
>                 dprintf("overrun: %d %d\n", len, max_message_len);
>                 *message_buffer = realloc(*message_buffer, len + 1);
>                 max_message_len = len + 1;
>         }
>         if (*message_buffer)
>                 return max_message_len;
>         else
>                 return 0;
> }
>
> The static declaration of max_message_len causes problems if
> multiple thread are calling this routine. Even worse. When opening
> a phone max_message_len must be 0. Otherwise gn_lib_phone_open()
> will fail.
>
> This makes the following code fail on the second invocation of
> gn_lib_phone_open(). max_message_len stays non-zero after
> the first call of verify_max_message_len() and therefor no memory
> will be allocated in the second call. *message_buffer will be NULL
> and gn_lib_phone_open() fails.
>
> Of course this only happens when the phonet-driver is used, so my
> serial nokia 6310 works fine.
>
> main(){
>   struct gn_statemachine *state;
>   gn_error err;
>
>   err=gn_lib_phoneprofile_load_from_file(GNOKIIRC, NULL, &state);
>   printf("load=%d, %s\n", err, gn_error_print(err));
>   err=gn_lib_phone_open(state);
>   printf("open=%d, %s\n", err, gn_error_print(err));
>   err=gn_lib_phone_close(state);
>   printf("close=%d, %s\n", err, gn_error_print(err));
>   err=gn_lib_phone_open(state); // err will be 9 with nokia 6230i and
> dku2libusb
>   printf("open=%d, %s\n", err, gn_error_print(err));
> }
>
> Here's my dirty fix for routine verify_max_message_len()
>
> static int verify_max_message_len(int len, char **message_buffer)
> {
>         static int max_message_len = 0;
>
>         if(len<PHONET_FRAME_MAX_LENGTH) len=PHONET_FRAME_MAX_LENGTH;
>         if(len>PHONET_FRAME_MAX_LENGTH || !*message_buffer){
>                 dprintf("reallocating message_buffer to %d bytes\n", len+1);
>                 *message_buffer = realloc(*message_buffer, len+1);
>         }
>         return *message_buffer ? len+1 : 0;
>
>         if (len > max_message_len) {
>                 dprintf("overrun: %d %d\n", len, max_message_len);
>                 *message_buffer = realloc(*message_buffer, len + 1);
>                 max_message_len = len + 1;
>         }
>         if (*message_buffer)
>                 return max_message_len;
>         else
>                 return 0;
> }
>
> Peter

Here's a completely untested patch. Care to give it a try?

Thank you.

diff --git a/common/links/fbus-phonet.c b/common/links/fbus-phonet.c
index 3300ceab..276de1d6 100644
--- a/common/links/fbus-phonet.c
+++ b/common/links/fbus-phonet.c
@@ -47,19 +47,18 @@ static gn_error phonet_send_message(unsigned int messagesize, unsigned char mess
 
 /*--------------------------------------------*/
 
-static int verify_max_message_len(int len, char **message_buffer)
+static int verify_max_message_len(int len, phonet_incoming_message *i)
 {
- static int max_message_len = 0;
-
- if (len > max_message_len || !*message_buffer) {
- dprintf("overrun, reallocating: %d %d\n", len, max_message_len);
- *message_buffer = realloc(*message_buffer, len + 1);
- max_message_len = len + 1;
+ if (len > i->message_buffer_size || !i->message_buffer) {
+ dprintf("overrun, reallocating: %d %d\n", len, i->message_buffer_size);
+ i->message_buffer_size = len + 1;
+ i->message_buffer = realloc(i->message_buffer, i->message_buffer_size);
  }
- if (*message_buffer)
- return max_message_len;
- else
- return 0;
+ if (i->message_buffer)
+ return i->message_buffer_size;
+
+ i->message_buffer_size = 0;
+ return 0;
 }
 
 
@@ -171,7 +170,7 @@ static void phonet_rx_statemachine(unsigned char rx_byte, struct gn_statemachine
  i->message_length = i->message_length + rx_byte;
  i->state = FBUS_RX_GetMessage;
  i->buffer_count = 0;
- if (!verify_max_message_len(i->message_length, &(i->message_buffer))) {
+ if (!verify_max_message_len(i->message_length, i)) {
  dprintf("PHONET: Failed to allocate memory for larger buffer\n");
  i->message_corrupted = 1;
  }
@@ -369,6 +368,7 @@ static void phonet_cleanup(struct gn_statemachine *state)
 {
  free(FBUSINST(state)->message_buffer);
  FBUSINST(state)->message_buffer = NULL;
+ FBUSINST(state)->message_buffer_size = 0;
 }
 
 /* Initialise variables and start the link */
@@ -388,7 +388,7 @@ gn_error phonet_initialise(struct gn_statemachine *state)
  if ((FBUSINST(state) = calloc(1, sizeof(phonet_incoming_message))) == NULL)
  return GN_ERR_MEMORYFULL;
 
- if (!verify_max_message_len(PHONET_FRAME_MAX_LENGTH, &(FBUSINST(state)->message_buffer))) {
+ if (!verify_max_message_len(PHONET_FRAME_MAX_LENGTH, FBUSINST(state))) {
  dprintf("PHONET: Failed to initalize initial incoming buffer for %d bytes\n", PHONET_FRAME_MAX_LENGTH);
  return GN_ERR_MEMORYFULL;
  }
diff --git a/include/links/fbus-phonet.h b/include/links/fbus-phonet.h
index 89eaf0ba..88eb06ca 100644
--- a/include/links/fbus-phonet.h
+++ b/include/links/fbus-phonet.h
@@ -48,6 +48,7 @@ typedef struct {
  int message_type;
  int message_length;
  char *message_buffer;
+ int message_buffer_size;
  int message_corrupted;
 } phonet_incoming_message;
 

_______________________________________________
gnokii-users mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/gnokii-users