Spurious warning when running with Ruby 3.2: "undefining the allocator of T_DATA class LibXML::XML::Writer" about libxml-ruby HOT 10 CLOSED

I think adding the following line in ruby_xml_writer.c should fix this:

rb_undef_alloc_func(cXMLWriter);

I could create a pull request, but I don't really know what the call does, and I'm not sure where to put it. Right after cXMLWriter = rb_define_class_under(...)? Or maybe after rb_undef_method(CLASS_OF(cXMLWriter), "new")?

from libxml-ruby.

I'd expect that some of the other uses of rb_define_class_under will produce the same warning under Ruby 3.2 and will need a similar fix, but I haven't tested these other classes.

from libxml-ruby.

That's interesting. So my understanding was calling rb_undef_alloc_func meant you can't create instances of that class - so any easy way to prevent someone creating an object that they should not because its created in C. For classes where you do want to allow object creation from Ruby then you almost always have to define an alloc function. Guess my understanding wasn't quite right.

Anyway, so yes, an MR like you propose sounds good.

from libxml-ruby.

I could create a PR that adds the rb_undef_alloc_func line in the couple of places I mentioned above, but I don't actually know what the line does, and I don't like writing code I don't understand. Maybe this line would introduce subtle bugs or memory leaks. I have no idea. I don't even know how Ruby objects are represented in C.

If you'll accept code written by someone who doesn't know what he's doing and can't even test the code, I'll create a PR. :-) But I'd rather not.

from libxml-ruby.

rb_undef_alloc_func removes the default alloc function which means you cannot call the new method anymore. So you can't create new instances of the class. Instead some other code in the extension needs to create them instead.

from libxml-ruby.

I created PR #202 that adds rb_undef_alloc_func for Writer, Reader and Schema. But I'm not at all sure it's correct. Please have a look.

First I created a simple Ruby file to check which classes produce this warning.

Note: I didn't check all libxml-ruby classes defined by rb_define_class. For example, I didn't check any of the XPath classes.

Result: Of the classes I checked, only Writer, Reader and Schema seem to produce this warning.

I also noticed that new is explicitly undefined for Writer, but not for Reader. If I understand correctly, this means that Reader still has the default implementation of new. Initially, calling Reader.new() works (but I guess it doesn't make much sense) and doesn't produce a warning. But calling another Reader constructor method, e.g. Reader.string("..."), will produce the warning undefining the allocator.... And after that, calling Reader.new() crashes with the error allocator undefined.

That behavior seemed confusing, so I also undefined new for Reader (with basically the same code as for Writer). Does that make sense? Is that OK? I'm not sure.

The new method for Schema is explicitly redefined, so I guess rb_undef_alloc_func is all that's needed for this class.

from libxml-ruby.

TODO:

• Do more libxml-ruby classes defined by rb_define_class produce the undefining the allocator warning?
• Is there a better way to find these classes?

from libxml-ruby.

Any class that has a new method that can be called from Ruby needs to have an allocator.

from libxml-ruby.

Any class that has a new method that can be called from Ruby needs to have an allocator.

Sounds reasonable, but is it true for all cases? When I ran my test script, I saw that LibXML::XML::Schema.new() produces the undefining the allocator warning when it is first called. When the Ruby runtime prints the warning, it also calls rb_undef_alloc_func(). If I understand correctly, the Schema class then doesn't have an allocator anymore. But calling LibXML::XML::Schema.new() still works.

I guess the actual rule is a bit more complex, e.g. "a class needs to have an allocator if it meets conditions X, Y and Z", where X might be "has a new method that can be called from Ruby", and Y and Z are qualifiers for special cases.

As I said, I don't really know what I'm doing here. But if I can contribute something useful without understanding all the details, I guess that's OK. :-)

from libxml-ruby.

I extended my test script. I think it now tests all libxml-ruby classes defined by rb_define_class.

I found that the undefining the allocator warning is also printed for LibXML::XML::RelaxNG constructor calls, so I created PR #205 that adds the rb_undef_alloc_func() call for the class.

Please have a look and check if that's OK. I don't know if this change might introduce subtle bugs, memory leaks, whatever...

from libxml-ruby.