[PATCH] Race between boxc_sender and smsbox identification

"Robert Galach" <[email protected]> Tue, 21 Jan 2003 15:25:38 +0100

Newsgroups	gmane.comp.mobile.kannel.devel
Message-ID	<[email protected]>

Hi again,

I found a problem in Kannel gateway (CVS version).
Let's check scenario. Consider bearerbox and 1 smsbox named "some_box_id".

1) Start bearerbox, it is waiting for smsboxes.
2) Start smsbox with smsbox_id = "some_box_id", connect to bearerbox.
3) In bearerbox struct Boxc is created and made connection to smsbox.
Default shared list for incoming_sms is set for this smsbox connection.
4) Bearebox starts boxc_sender thread for connected smsbox and sender waits
for incoming_sms in shared list until some msg will arrive.
5) Smsbox identify itself as "some_box_id" and new private list for
incoming_sms is created, but previous cannot be destroyed because it is
shared.

But sender is still waiting on shared list!!! And it will wait forever. No
sms can be sent by smsbox "some_box_id".

I have patched this bug in local version by adding mutex to struct boxc and
forcing boxc_sender to wait on this mutex until smsbox will identify itself.
Then sender starts his real job.
Changed also smsbox.c to send cmd_identify even if boxc_id is NULL.

The patch is attached. I hope it removes problem.

Regards
Robert Galach

smsbox.c.patch (application/octet-stream, 758 B)

Index: gw/smsbox.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsbox.c,v
retrieving revision 1.215
diff -u -r1.215 smsbox.c
--- gw/smsbox.c 31 Dec 2002 17:40:57 -0000      1.215
+++ gw/smsbox.c 21 Jan 2003 14:07:11 -0000
@@ -86,12 +86,11 @@
 {
     Msg *msg;

-    if (smsbox_id != NULL) {
-        msg = msg_create(admin);
-        msg->admin.command = cmd_identify;
-        msg->admin.boxc_id = octstr_duplicate(smsbox_id);
-        write_to_bearerbox(msg);
-    }
+    /* identify even if smsbox_id is not set */
+    msg = msg_create(admin);
+    msg->admin.command = cmd_identify;
+    msg->admin.boxc_id = smsbox_id != NULL ? octstr_duplicate(smsbox_id) : NULL;
+    write_to_bearerbox(msg);
 }

bb_boxc.c.patch (application/octet-stream, 2.6 KB)

Index: gw/bb_boxc.c
===================================================================
RCS file: /home/cvs/gateway/gw/bb_boxc.c,v
retrieving revision 1.59
diff -u -r1.59 bb_boxc.c
--- gw/bb_boxc.c        14 Nov 2002 02:29:25 -0000      1.59
+++ gw/bb_boxc.c        21 Jan 2003 14:03:49 -0000
@@ -71,6 +71,7 @@
     List               *outgoing;
     volatile sig_atomic_t alive;
     Octstr *boxc_id; /* identifies the connected smsbox instance */
+    Mutex *boxc_id_mutex; /* stops boxc_sender until smsbox identification*/
 } Boxc;


@@ -187,6 +188,8 @@
             }
             /* if this is an identification message from an smsbox instance */
             else if (msg_type(msg) == admin && msg->admin.command == cmd_identify) {
+                /* now smsbox sends this command even if boxc_id is NULL */
+                if (msg->admin.boxc_id != NULL) {
                 List *newlist;

                 /* and add the boxc_ud into conn for boxc_status() output */
@@ -205,6 +208,9 @@
                 debug("bb.boxc", 0, "boxc_receiver: got boxc_id <%s> from <%s>",
                 octstr_get_cstr(msg->admin.boxc_id),
                 octstr_get_cstr(conn->client_ip));
+                }
+                debug("bb.boxc", 0, "boxc_receiver: unlocking sender");
+                mutex_unlock(conn->boxc_id_mutex);
             }
             else
                 warning(0, "boxc_receiver: unknown msg received from <%s>, "
@@ -224,6 +230,7 @@
     Octstr *pack;

     pack = msg_pack(pmsg);
+    debug("bb.boxc", 0, "send_msg: trying to send to boxc: <%s>", octstr_get_cstr(boxconn->boxc_id));
     if (conn_write_withlen(boxconn->conn, pack) == -1) {
        error(0, "Couldn't write Msg to box <%s>, disconnecting",
              octstr_get_cstr(boxconn->client_ip));
@@ -241,6 +248,12 @@

     list_add_producer(flow_threads);

+    /* wait for smsbox identification */
+    if (bb_status != BB_DEAD && conn->alive) {
+        mutex_lock(conn->boxc_id_mutex);
+        debug("bb.boxc", 0, "boxc_sender: sender unlocked");
+    }
+
     while(bb_status != BB_DEAD && conn->alive) {

        /* Make sure there's no data left in the outgoing connection before
@@ -303,6 +316,8 @@
     boxc->client_ip = ip;
     boxc->alive = 1;
     boxc->connect_time = time(NULL);
+    boxc->boxc_id_mutex = mutex_create();
+    mutex_lock(boxc->boxc_id_mutex);
     boxc->boxc_id = NULL;
     return boxc;
 }
@@ -318,6 +333,7 @@
            conn_destroy(boxc->conn);
     octstr_destroy(boxc->client_ip);
     octstr_destroy(boxc->boxc_id);
+    mutex_destroy(boxc->boxc_id_mutex);
     gw_free(boxc);
 }