FS-6403 --resolve

This commit also reverts 2 previous attempts to fix this very rare race issue spanning back to 2009

62ce8538974f727778f1024d0ef9549e438704fe Patch from MOC
3a85348cdfd0fd7df63a2a150790722c2d294b36 FS-2302 mutex added around switch_xml_toxml()

The real problem was switch_xml_toxml_buf() was actually temporarily modifying the xml structure being searialized to make it appaer to be a root structure then serializing it and restoring the pointers.  This caused a non-threadsafe operation when some other thread was scanning the same xml structure.

This patch removes the modification and instead passes a new arg to switch_xml_toxml_r indicating to treat the structure as if it were a root structure.

This bug has been present since the induction of xml into FS.

Conflicts:
	src/switch_xml.c

diff --git a/src/switch_xml.c b/src/switch_xml.c
index 614ed178c113cb7a47aaf9ed01081b14c94db633..d809bcee3ee57ca7df160266311c75b6b317935f 100644 (file)
--- a/src/switch_xml.c
+++ b/src/switch_xml.c
@@ -179,7 +179,6 @@ static switch_mutex_t *XML_LOCK = NULL;
 static switch_mutex_t *CACHE_MUTEX = NULL;
 static switch_mutex_t *REFLOCK = NULL;
 static switch_mutex_t *FILE_LOCK = NULL;
-static switch_mutex_t *XML_GEN_LOCK = NULL;
 
 SWITCH_DECLARE_NONSTD(switch_xml_t) __switch_xml_open_root(uint8_t reload, const char **err, void *user_data);
 
@@ -446,11 +445,6 @@ SWITCH_DECLARE(const char *) switch_xml_attr(switch_xml_t xml, const char *attr)
        while (root->xml.parent)
                root = (switch_xml_root_t) root->xml.parent;    /* root tag */
 
-       /* Make sure root is really a switch_xml_root_t (Issues with switch_xml_toxml) */
-       if (!root->xml.is_switch_xml_root_t) {
-               return NULL;
-       }
-
        if (!root->attr) {
                return NULL;
        }
@@ -2351,7 +2345,6 @@ SWITCH_DECLARE(switch_status_t) switch_xml_init(switch_memory_pool_t *pool, cons
        switch_mutex_init(&XML_LOCK, SWITCH_MUTEX_NESTED, XML_MEMORY_POOL);
        switch_mutex_init(&REFLOCK, SWITCH_MUTEX_NESTED, XML_MEMORY_POOL);
        switch_mutex_init(&FILE_LOCK, SWITCH_MUTEX_NESTED, XML_MEMORY_POOL);
-       switch_mutex_init(&XML_GEN_LOCK, SWITCH_MUTEX_NESTED, XML_MEMORY_POOL);
        switch_core_hash_init(&CACHE_HASH, XML_MEMORY_POOL);
        switch_core_hash_init(&CACHE_EXPIRES_HASH, XML_MEMORY_POOL);
 
@@ -2519,17 +2512,26 @@ static char *switch_xml_ampencode(const char *s, switch_size_t len, char **dst,
 /* Recursively converts each tag to xml appending it to *s. Reallocates *s if
    its length exceeds max. start is the location of the previous tag in the
    parent tag's character content. Returns *s. */
-static char *switch_xml_toxml_r(switch_xml_t xml, char **s, switch_size_t *len, switch_size_t *max, switch_size_t start, char ***attr, uint32_t *count)
+static char *switch_xml_toxml_r(switch_xml_t xml, char **s, switch_size_t *len, switch_size_t *max, switch_size_t start, char ***attr, uint32_t *count, int isroot)
 {
        int i, j;
        char *txt;
        switch_size_t off;
        uint32_t lcount;
+       uint32_t loops = 0;
 
   tailrecurse:
        off = 0;
        lcount = 0;
-       txt = (char *) (xml->parent) ? xml->parent->txt : (char *) "";
+       txt = "";
+
+       if (loops++) {
+               isroot = 0;
+       }
+
+       if (!isroot && xml->parent) {
+               txt = (char *) xml->parent->txt;
+       }
 
        /* parent character content up to this tag */
        *s = switch_xml_ampencode(txt + start, xml->off - start, s, len, max, 0);
@@ -2584,7 +2586,7 @@ static char *switch_xml_toxml_r(switch_xml_t xml, char **s, switch_size_t *len,
 
        if (xml->child) {
                (*count)++;
-               *s = switch_xml_toxml_r(xml->child, s, len, max, 0, attr, count);
+               *s = switch_xml_toxml_r(xml->child, s, len, max, 0, attr, count, 0);
 
        } else {
                *s = switch_xml_ampencode(xml->txt, 0, s, len, max, 0); /* data */
@@ -2609,7 +2611,7 @@ static char *switch_xml_toxml_r(switch_xml_t xml, char **s, switch_size_t *len,
        while (txt[off] && off < xml->off)
                off++;                                  /* make sure off is within bounds */
 
-       if (xml->ordered) {
+       if (!isroot && xml->ordered) {
                xml = xml->ordered;
                start = off;
                goto tailrecurse;
@@ -2638,9 +2640,8 @@ SWITCH_DECLARE(char *) switch_xml_toxml(switch_xml_t xml, switch_bool_t prn_head
        s = (char *) malloc(SWITCH_XML_BUFSIZE);
        switch_assert(s);
 
-       switch_mutex_lock(XML_GEN_LOCK);
        r = switch_xml_toxml_buf(xml, s, SWITCH_XML_BUFSIZE, 0, prn_header);
-       switch_mutex_unlock(XML_GEN_LOCK);
+
        return r;
 }
 
@@ -2649,7 +2650,6 @@ SWITCH_DECLARE(char *) switch_xml_tohtml(switch_xml_t xml, switch_bool_t prn_hea
        char *r, *s, *h;
        switch_size_t rlen = 0;
        switch_size_t len = SWITCH_XML_BUFSIZE;
-       switch_mutex_lock(XML_GEN_LOCK);
        s = (char *) malloc(SWITCH_XML_BUFSIZE);
        switch_assert(s);
        h = (char *) malloc(SWITCH_XML_BUFSIZE);
@@ -2657,7 +2657,6 @@ SWITCH_DECLARE(char *) switch_xml_tohtml(switch_xml_t xml, switch_bool_t prn_hea
        r = switch_xml_toxml_buf(xml, s, SWITCH_XML_BUFSIZE, 0, prn_header);
        h = switch_xml_ampencode(r, 0, &h, &rlen, &len, 1);
        switch_safe_free(r);
-       switch_mutex_unlock(XML_GEN_LOCK);
        return h;
 }
 
@@ -2665,7 +2664,7 @@ SWITCH_DECLARE(char *) switch_xml_tohtml(switch_xml_t xml, switch_bool_t prn_hea
    must be freed */
 SWITCH_DECLARE(char *) switch_xml_toxml_buf(switch_xml_t xml, char *buf, switch_size_t buflen, switch_size_t offset, switch_bool_t prn_header)
 {
-       switch_xml_t p = (xml) ? xml->parent : NULL, o = (xml) ? xml->ordered : NULL;
+       switch_xml_t p = (xml) ? xml->parent : NULL;
        switch_xml_root_t root = (switch_xml_root_t) xml;
        switch_size_t len = 0, max = buflen;
        char *s, *t, *n, *r;
@@ -2707,10 +2706,7 @@ SWITCH_DECLARE(char *) switch_xml_toxml_buf(switch_xml_t xml, char *buf, switch_
                }
        }
 
-       xml->parent = xml->ordered = NULL;
-       s = switch_xml_toxml_r(xml, &s, &len, &max, 0, root->attr, &count);
-       xml->parent = p;
-       xml->ordered = o;
+       s = switch_xml_toxml_r(xml, &s, &len, &max, 0, root->attr, &count, 1);
 
        for (i = 0; !p && root->pi[i]; i++) {   /* post-root processing instructions */
                for (k = 2; root->pi[i][k - 1]; k++);
@@ -2843,7 +2839,6 @@ SWITCH_DECLARE(switch_xml_t) switch_xml_new(const char *name)
        strcpy(root->err, root->xml.txt = (char *) "");
        root->ent = (char **) memcpy(malloc(sizeof(ent)), ent, sizeof(ent));
        root->attr = root->pi = (char ***) (root->xml.attr = SWITCH_XML_NIL);
-       root->xml.is_switch_xml_root_t = SWITCH_TRUE;
        return &root->xml;
 }