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
This commit is contained in:
Anthony Minessale 2014-04-03 20:17:16 +05:00
parent 1936bdd410
commit 9836564352

View File

@ -179,7 +179,6 @@ static switch_mutex_t *XML_LOCK = NULL;
static switch_mutex_t *CACHE_MUTEX = NULL; static switch_mutex_t *CACHE_MUTEX = NULL;
static switch_mutex_t *REFLOCK = NULL; static switch_mutex_t *REFLOCK = NULL;
static switch_mutex_t *FILE_LOCK = 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); 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) while (root->xml.parent)
root = (switch_xml_root_t) root->xml.parent; /* root tag */ 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) { if (!root->attr) {
return NULL; 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(&XML_LOCK, SWITCH_MUTEX_NESTED, XML_MEMORY_POOL);
switch_mutex_init(&REFLOCK, 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(&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_HASH, XML_MEMORY_POOL);
switch_core_hash_init(&CACHE_EXPIRES_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 /* 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 its length exceeds max. start is the location of the previous tag in the
parent tag's character content. Returns *s. */ 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; int i, j;
char *txt; char *txt;
switch_size_t off; switch_size_t off;
uint32_t lcount; uint32_t lcount;
uint32_t loops = 0;
tailrecurse: tailrecurse:
off = 0; off = 0;
lcount = 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 */ /* parent character content up to this tag */
*s = switch_xml_ampencode(txt + start, xml->off - start, s, len, max, 0); *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) { if (xml->child) {
(*count)++; (*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 { } else {
*s = switch_xml_ampencode(xml->txt, 0, s, len, max, 0); /* data */ *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) while (txt[off] && off < xml->off)
off++; /* make sure off is within bounds */ off++; /* make sure off is within bounds */
if (xml->ordered) { if (!isroot && xml->ordered) {
xml = xml->ordered; xml = xml->ordered;
start = off; start = off;
goto tailrecurse; 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); s = (char *) malloc(SWITCH_XML_BUFSIZE);
switch_assert(s); switch_assert(s);
switch_mutex_lock(XML_GEN_LOCK);
r = switch_xml_toxml_buf(xml, s, SWITCH_XML_BUFSIZE, 0, prn_header); r = switch_xml_toxml_buf(xml, s, SWITCH_XML_BUFSIZE, 0, prn_header);
switch_mutex_unlock(XML_GEN_LOCK);
return r; 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; char *r, *s, *h;
switch_size_t rlen = 0; switch_size_t rlen = 0;
switch_size_t len = SWITCH_XML_BUFSIZE; switch_size_t len = SWITCH_XML_BUFSIZE;
switch_mutex_lock(XML_GEN_LOCK);
s = (char *) malloc(SWITCH_XML_BUFSIZE); s = (char *) malloc(SWITCH_XML_BUFSIZE);
switch_assert(s); switch_assert(s);
h = (char *) malloc(SWITCH_XML_BUFSIZE); 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); r = switch_xml_toxml_buf(xml, s, SWITCH_XML_BUFSIZE, 0, prn_header);
h = switch_xml_ampencode(r, 0, &h, &rlen, &len, 1); h = switch_xml_ampencode(r, 0, &h, &rlen, &len, 1);
switch_safe_free(r); switch_safe_free(r);
switch_mutex_unlock(XML_GEN_LOCK);
return h; return h;
} }
@ -2665,7 +2664,7 @@ SWITCH_DECLARE(char *) switch_xml_tohtml(switch_xml_t xml, switch_bool_t prn_hea
must be freed */ 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_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_xml_root_t root = (switch_xml_root_t) xml;
switch_size_t len = 0, max = buflen; switch_size_t len = 0, max = buflen;
char *s, *t, *n, *r; 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, 1);
s = switch_xml_toxml_r(xml, &s, &len, &max, 0, root->attr, &count);
xml->parent = p;
xml->ordered = o;
for (i = 0; !p && root->pi[i]; i++) { /* post-root processing instructions */ for (i = 0; !p && root->pi[i]; i++) { /* post-root processing instructions */
for (k = 2; root->pi[i][k - 1]; k++); 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 *) ""); strcpy(root->err, root->xml.txt = (char *) "");
root->ent = (char **) memcpy(malloc(sizeof(ent)), ent, sizeof(ent)); root->ent = (char **) memcpy(malloc(sizeof(ent)), ent, sizeof(ent));
root->attr = root->pi = (char ***) (root->xml.attr = SWITCH_XML_NIL); root->attr = root->pi = (char ***) (root->xml.attr = SWITCH_XML_NIL);
root->xml.is_switch_xml_root_t = SWITCH_TRUE;
return &root->xml; return &root->xml;
} }