Fix deadlock and non-working socket msg throttling

Two major bugs in btif_config.c.  One is that due to improper
locking order, a deadlock could occur (symptom is generally
out of GKI buffers because BTIF thread stops processing it's
mailbox because it is blocked sending a msg to the socket
thread, which in turn is blocked because BTIF is holding
the lock it needs to do the config save that BTIF requested
in the first place).

The other is also lock related.  The mechanism to throttle
socket messages based on whether there was one already
in flight wasn't working because there wasn't locking
when that variable was being used.  The compiler would be
free to cache it in a register on SMP machines, so it would
never look like there as a value in flight.  This would
cause a bunch of messages to be queued up, and then the
way the handling worked in cfg_cmd_callback(), every
cmd would cause a 3 second sleep, even if there was nothing
to do.  Modified the loop to check if there was nothing
to do before sleeping.

I was running into both bugs when repeatedly pairing and
unpairing a BLE device.  The deadlock would hit first, causing
out of GKI buffers.  Once I fixed the deadlock, I would still
run out of GKI buffers because the socket thread wasn't working
fast enough to clear the backlog of socket messages it had
received.

Also optimized some string termination code which was using
a memset of the full buffer before and then copying over
every entry except the last.  Now we do the copy and just
set the last buffer entry to null.

Change-Id: Ic06ef0b8e15d8f1fe669fb88439851ffbad560de
Signed-off-by: Mike J. Chen <mjchen@google.com>

diff --git a/btif/src/btif_config.c b/btif/src/btif_config.c
index 33f0239..7b5ab07 100644 (file)
--- a/btif/src/btif_config.c
+++ b/btif/src/btif_config.c
@@ -89,7 +89,7 @@ static pthread_mutex_t slot_lock;
 static int pth = -1; //poll thread handle
 static cfg_node root;
 static int cached_change;
-static int processing_save_cmd;
+static int save_cmds_queued;
 static void cfg_cmd_callback(int cmd_fd, int type, int flags, uint32_t user_id);
 static inline short alloc_node(cfg_node* p, short grow);
 static inline void free_node(cfg_node* p);
@@ -337,11 +337,22 @@ int btif_config_enum(btif_config_enum_callback cb, void* user_data)
 }
 int btif_config_save()
 {
+    int post_cmd = 0;
     lock_slot(&slot_lock);
-    bdld("processing_save_cmd:%d, cached change:%d", processing_save_cmd, cached_change);
-    if(!processing_save_cmd && cached_change > 0)
-        btsock_thread_post_cmd(pth, CFG_CMD_SAVE, NULL, 0, 0);
+    bdld("save_cmds_queued:%d, cached_change:%d", save_cmds_queued, cached_change);
+    if((save_cmds_queued == 0) && (cached_change > 0))
+    {
+        post_cmd = 1;
+        save_cmds_queued++;
+        bdld("post_cmd set to 1, save_cmds_queued:%d", save_cmds_queued);
+    }
     unlock_slot(&slot_lock);
+    /* don't hold lock when invoking send or else a deadlock could
+     * occur when the socket thread tries to do the actual saving.
+     */
+    if (post_cmd)
+        btsock_thread_post_cmd(pth, CFG_CMD_SAVE, NULL, 0, 0);
+
     return TRUE;
 }
 void btif_config_flush()
@@ -689,7 +700,6 @@ static int save_cfg()
    if(btif_config_save_file(file_name_new))
     {
         cached_change = 0;
-        processing_save_cmd = 0;
         chown(file_name_new, -1, AID_NET_BT_STACK);
         chmod(file_name_new, 0660);
         rename(file_name, file_name_old);
@@ -784,25 +794,34 @@ static void cfg_cmd_callback(int cmd_fd, int type, int size, uint32_t user_id)
     UNUSED(size);
     UNUSED(user_id);
 
-  //bdld("cmd type:%d, size:%d", type, size);
     switch(type)
     {
         case CFG_CMD_SAVE:
         {
-            int last_cached_change = cached_change;
-            processing_save_cmd = 1;
+            int i;
+            int last_cached_change;
+
+            // grab lock while accessing cached_change.
+            lock_slot(&slot_lock);
+            bdla(save_cmds_queued > 0);
+            save_cmds_queued--;
+            last_cached_change = cached_change;
             //hold the file saving until no more change in last 3 seconds.
             bdld("wait until no more changes in short time, cached change:%d", cached_change);
-            int i;
-            for(i = 0; i < 100; i ++) //5 minitue max waiting
+            for(i = 0; i < 100; i ++) //5 minutes max waiting
             {
+                // don't sleep if there is nothing to do
+                if(cached_change == 0)
+                    break;
+                // release lock during sleep
+                unlock_slot(&slot_lock);
                 sleep(3);
-                if(cached_change == 0 || last_cached_change == cached_change)
+                lock_slot(&slot_lock);
+                if(last_cached_change == cached_change)
                     break;
                 last_cached_change = cached_change;
             }
             bdld("writing the bt_config.xml now, cached change:%d", cached_change);
-            lock_slot(&slot_lock);
             if(cached_change > 0)
                 save_cfg();
             unlock_slot(&slot_lock);