Skip to content

Commit 681b3fa

Browse files
committed
Update libxml-ruby to no longer use the _private field in XML objects. Instead maintain a separate registry that maps XML objects to Ruby values.
1 parent 0a3273d commit 681b3fa

11 files changed

Lines changed: 116 additions & 40 deletions

ext/libxml/libxml.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ void Init_libxml_ruby(void)
4646
mLibXML = rb_define_module("LibXML");
4747

4848
rxml_init_memory();
49+
rxml_init_registry();
4950
rxml_init_xml();
5051
rxml_init_io();
5152
rxml_init_error();

ext/libxml/ruby_libxml.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "ruby_xml_dtd.h"
3838
#include "ruby_xml_schema.h"
3939
#include "ruby_xml_relaxng.h"
40+
#include "ruby_xml_registry.h"
4041

4142
extern VALUE mLibXML;
4243

ext/libxml/ruby_xml_attr.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ static VALUE rxml_attr_doc_get(VALUE self)
140140
if (xattr->doc == NULL)
141141
return Qnil;
142142
else
143-
return rxml_document_wrap(xattr->doc);
143+
return rxml_registry_lookup(xattr->doc);
144144
}
145145

146146
/*

ext/libxml/ruby_xml_attr_decl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ static VALUE rxml_attr_decl_doc_get(VALUE self)
4343
if (xattr->doc == NULL)
4444
return Qnil;
4545
else
46-
return rxml_document_wrap(xattr->doc);
46+
return rxml_registry_lookup(xattr->doc);
4747
}
4848

4949

ext/libxml/ruby_xml_document.c

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ void rxml_document_free(void* data)
6464
{
6565
xmlDocPtr xdoc = (xmlDocPtr)data;
6666
if (!xdoc) return;
67-
xdoc->_private = NULL;
67+
rxml_registry_unregister(xdoc);
6868
xmlFreeDoc(xdoc);
6969
}
7070

@@ -78,15 +78,12 @@ VALUE rxml_document_wrap(xmlDocPtr xdoc)
7878
{
7979
VALUE result = Qnil;
8080

81-
// Is this node is already wrapped?
82-
if (xdoc->_private != NULL)
83-
{
84-
result = (VALUE)xdoc->_private;
85-
}
86-
else
81+
// Is this document already wrapped?
82+
result = rxml_registry_lookup(xdoc);
83+
if (NIL_P(result))
8784
{
8885
result = TypedData_Wrap_Struct(cXMLDocument, &rxml_document_data_type, xdoc);
89-
xdoc->_private = (void*)result;
86+
rxml_registry_register(xdoc, result);
9087
}
9188

9289
return result;
@@ -133,7 +130,7 @@ static VALUE rxml_document_initialize(int argc, VALUE *argv, VALUE self)
133130

134131
// Link the ruby object to the document and the document to the ruby object
135132
RTYPEDDATA_DATA(self) = xdoc;
136-
xdoc->_private = (void*)self;
133+
rxml_registry_register(xdoc, self);
137134

138135
return self;
139136
}

ext/libxml/ruby_xml_dtd.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ static void rxml_dtd_mark(void* data)
4747
xmlDtdPtr xdtd = (xmlDtdPtr)data;
4848
if (xdtd && xdtd->doc)
4949
{
50-
VALUE doc = (VALUE)xdtd->doc->_private;
51-
rb_gc_mark(doc);
50+
VALUE doc = rxml_registry_lookup(xdtd->doc);
51+
if (!NIL_P(doc))
52+
rb_gc_mark(doc);
5253
}
5354
}
5455

ext/libxml/ruby_xml_node.c

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ VALUE cXMLNode;
3030
* have a parent and do not belong to a document). In these cases,
3131
* the bindings manage the memory. They do this by installing a free
3232
* function and storing a back pointer to the Ruby object from the xmlnode
33-
* using the _private member on libxml structures. When the Ruby object
34-
* goes out of scope, the underlying libxml structure is freed. Libxml
35-
* itself then frees all child node (recursively).
33+
* using a pointer-keyed registry (see ruby_xml_registry.c). When the
34+
* Ruby object goes out of scope, the underlying libxml structure is freed.
35+
* Libxml itself then frees all child node (recursively).
3636
*
3737
* For all other nodes (the vast majority), the bindings create temporary
3838
* Ruby objects that get freed once they go out of scope. Thus there can be
@@ -60,8 +60,7 @@ static void rxml_node_free(void *data)
6060
responsible for freeing the underlying node.*/
6161
if (xnode->doc == NULL && xnode->parent == NULL)
6262
{
63-
// Remove the back linkage from libxml to Ruby
64-
xnode->_private = NULL;
63+
rxml_registry_unregister(xnode);
6564
xmlFreeNode(xnode);
6665
}
6766
}
@@ -108,14 +107,14 @@ void rxml_node_manage(xmlNodePtr xnode, VALUE node)
108107
{
109108
VALUE *type_ptr = (VALUE *)&RTYPEDDATA(node)->type;
110109
*type_ptr = (*type_ptr & TYPED_DATA_EMBEDDED) | (VALUE)&rxml_node_managed_data_type;
111-
xnode->_private = (void*)node;
110+
rxml_registry_register(xnode, node);
112111
}
113112

114113
void rxml_node_unmanage(xmlNodePtr xnode, VALUE node)
115114
{
116115
VALUE *type_ptr = (VALUE *)&RTYPEDDATA(node)->type;
117116
*type_ptr = (*type_ptr & TYPED_DATA_EMBEDDED) | (VALUE)&rxml_node_data_type;
118-
xnode->_private = NULL;
117+
rxml_registry_unregister(xnode);
119118
}
120119

121120
xmlNodePtr rxml_node_root(xmlNodePtr xnode)
@@ -134,20 +133,16 @@ void rxml_node_mark(xmlNodePtr xnode)
134133
{
135134
if (xnode->doc)
136135
{
137-
if (xnode->doc->_private)
138-
{
139-
VALUE doc = (VALUE)xnode->doc->_private;
140-
rb_gc_mark(doc);
141-
}
136+
VALUE doc = rxml_registry_lookup(xnode->doc);
137+
if (!NIL_P(doc))
138+
rb_gc_mark(doc);
142139
}
143140
else if (xnode->parent)
144141
{
145142
xmlNodePtr root = rxml_node_root(xnode);
146-
if (root->_private)
147-
{
148-
VALUE node = (VALUE)root->_private;
143+
VALUE node = rxml_registry_lookup(root);
144+
if (!NIL_P(node))
149145
rb_gc_mark(node);
150-
}
151146
}
152147
}
153148

@@ -156,11 +151,8 @@ VALUE rxml_node_wrap(xmlNodePtr xnode)
156151
VALUE result = Qnil;
157152

158153
// Is this node already wrapped?
159-
if (xnode->_private)
160-
{
161-
result = (VALUE)xnode->_private;
162-
}
163-
else
154+
result = rxml_registry_lookup(xnode);
155+
if (NIL_P(result))
164156
{
165157
result = TypedData_Wrap_Struct(cXMLNode, &rxml_node_data_type, xnode);
166158
}
@@ -580,7 +572,7 @@ static VALUE rxml_node_doc(VALUE self)
580572
if (xdoc == NULL)
581573
return (Qnil);
582574

583-
return (VALUE)xdoc->_private;
575+
return rxml_registry_lookup(xdoc);
584576
}
585577

586578
/*

ext/libxml/ruby_xml_reader.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,11 @@ static void rxml_reader_mark(void* data)
7070
{
7171
xmlTextReaderPtr xreader = (xmlTextReaderPtr)data;
7272
xmlDocPtr xdoc = xmlTextReaderCurrentDoc(xreader);
73-
if (xdoc && xdoc->_private)
73+
if (xdoc)
7474
{
75-
VALUE doc = (VALUE)xdoc->_private;
76-
rb_gc_mark(doc);
75+
VALUE doc = rxml_registry_lookup(xdoc);
76+
if (!NIL_P(doc))
77+
rb_gc_mark(doc);
7778
}
7879
}
7980

ext/libxml/ruby_xml_registry.c

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/* Please see the LICENSE file for copyright and distribution information */
2+
3+
#include "ruby_libxml.h"
4+
#include "ruby_xml_registry.h"
5+
6+
#include <ruby/st.h>
7+
8+
static st_table *rxml_registry;
9+
10+
void rxml_init_registry(void)
11+
{
12+
rxml_registry = st_init_numtable();
13+
}
14+
15+
void rxml_registry_register(void *ptr, VALUE obj)
16+
{
17+
st_insert(rxml_registry, (st_data_t)ptr, (st_data_t)obj);
18+
}
19+
20+
void rxml_registry_unregister(void *ptr)
21+
{
22+
st_delete(rxml_registry, (st_data_t *)&ptr, NULL);
23+
}
24+
25+
VALUE rxml_registry_lookup(void *ptr)
26+
{
27+
st_data_t val;
28+
if (st_lookup(rxml_registry, (st_data_t)ptr, &val))
29+
return (VALUE)val;
30+
return Qnil;
31+
}

ext/libxml/ruby_xml_registry.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/* Please see the LICENSE file for copyright and distribution information */
2+
3+
#ifndef __RXML_REGISTRY__
4+
#define __RXML_REGISTRY__
5+
6+
/* Pointer-to-VALUE registry for mapping C structs (xmlDocPtr, xmlNodePtr,
7+
etc.) back to their Ruby wrappers.
8+
9+
The registry is a global st_table keyed by C pointer address. It is NOT a
10+
GC root -- entries do not prevent garbage collection. Objects stay alive
11+
through the normal mark chains (mark functions look up the registry instead
12+
of reading _private).
13+
14+
Registered pointers MUST be unregistered before the underlying C struct is
15+
freed, typically in the wrapper's dfree function. */
16+
17+
void rxml_init_registry(void);
18+
void rxml_registry_register(void *ptr, VALUE obj);
19+
void rxml_registry_unregister(void *ptr);
20+
VALUE rxml_registry_lookup(void *ptr); /* Qnil on miss */
21+
22+
#endif

0 commit comments

Comments
 (0)