Feature #20265: Deprecate and remove rb_newobj and rb_newobj_of

• Status: Closed
• Assignee: peterzhu2118 (Peter Zhu)
• Target version: -

[peterzhu2118 (Peter Zhu)]

GitHub PR: https://github.com/ruby/ruby/pull/9964

I’m proposing deprecating and removing the rb_newobj and rb_newobj_of APIs because they are difficult to use, fragile to use, and requires knowledge of the internal implementation of data types in Ruby.

The rb_newobj function creates a T_NONE object. T_NONE objects are tricky to deal with since T_NONE objects cannot be marked, T_NONE objects are not reclaimed by the GC, and changing the object to other types require internal knowledge about the data type.

T_NONE objects are not allowed to be marked, so it cannot be GC managed. Since T_NONE objects are skipped during sweeping, it will leak Ruby heap memory if the developer never changes the object to another type.

Changing a T_NONE object to another type is tricky. For example, T_STRING objects have many flags for embedded, shared, shared root, encoding, coderange, etc. Many of these flags are not public, preventing direct use by developers. Developers must understand these flags to convert a T_NONE object into a T_STRING object.

While the rb_newobj_of function is easier to use than the rb_newobj function, it still requires developers to understand flags, meaning some issues of rb_newobj also apply to rb_newobj_of.

Below is the usage of RB_NEWOBJ, rb_newobj, rb_newobj_of, RB_NEWOBJ_OF with vendored Ruby and comments removed. You can see that there are very few gems using these APIs and all are from over a decade ago (the most recent one is from 2011).

2009-11-18 /srv/gems/bleak_house-7.2/ruby/ruby-1.8.7.patch:@@ -438,10 +438,8 @@ rb_newobj()
2023-07-01 /srv/gems/daqing_rucaptcha-3.2.2/ext/rucaptcha/target/release/build/rb-sys-6bdd5b2895b9570a/out/bindings-0.9.78-arm64-darwin22-3.2.2.rs:    pub fn rb_newobj() -> VALUE;
2023-07-01 /srv/gems/daqing_rucaptcha-3.2.2/ext/rucaptcha/target/release/build/rb-sys-6bdd5b2895b9570a/out/bindings-0.9.78-arm64-darwin22-3.2.2.rs:    pub fn rb_newobj_of(klass: VALUE, flags: VALUE) -> VALUE;
2010-08-06 /srv/gems/langscan-1.2/ext/langscan/ruby/compat/ripper/ripper.c:    NODE *n = (NODE*)rb_newobj();
2011-02-03 /srv/gems/memprof-0.3.10/ext/memprof.c:  VALUE ret = rb_newobj();
2011-02-03 /srv/gems/memprof-0.3.10/ext/tracers/objects.c:  last_obj = orig_rb_newobj();
2006-02-28 /srv/gems/refe-0.8.0.3/data/refe/function_source/rb/newobj:rb_newobj()
2006-02-28 /srv/gems/refe-0.8.0.3/data/refe/function_source/rb/node/newnode:    NODE *n = (NODE*)rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/dependency.c:       prov = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/dependency.c:       req = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/dependency.c:       conf = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/dependency.c:       obso = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/file.c:     file = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/source.c:   src = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/source.c:   src = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/source.c:   src = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/version.c:  ver = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/version.c:  ver = rb_newobj();
2011-08-31 /srv/gems/ruby-rpm-1.3.1/ext/rpm/version.c:  ver = rb_newobj();
2006-11-30 /srv/gems/sydparse-1.2.0/sydparse.c:    syd_scope = (struct SCOPE*)rb_newobj();
2006-11-30 /srv/gems/sydparse-1.2.0/sydparse.y:    syd_scope = (struct SCOPE*)rb_newobj();

[shyouhei (Shyouhei Urabe)]

I'm positive. C extensions should basically use TypedData_Make_Struct instead.

[Eregon (Benoit Daloze)]

+1

[mame (Yusuke Endoh)]

• Status changed from Open to Assigned
• Assignee set to peterzhu2118 (Peter Zhu)

Discussed at the dev meeting, and @matz (Yukihiro Matsumoto) said "go ahead".

[peterzhu2118 (Peter Zhu)]

• Status changed from Assigned to Closed

Applied in changeset git|8e1831406ffd385213f67baba0d1fe6d44e9e0ea.

---

[Feature #20265] Remove rb_newobj and RB_NEWOBJ

[vo.x (Vit Ondruch)]

Just FTR, there are also project which are using NEWOBJ, such as:

https://github.com/search?q=repo%3Ahparra%2Fruby-serialport%20NEWOBJ&type=code

So what is recommended instead?

[peterzhu2118 (Peter Zhu)]

Just FTR, there are also project which are using NEWOBJ, such as:

https://github.com/search?q=repo%3Ahparra%2Fruby-serialport%20NEWOBJ&type=code

So what is recommended instead?

That code is using internal data structures which are subject to change. It should use an API such as rb_io_open_descriptor instead.

[vo.x (Vit Ondruch)]

That does not have too long history, seeing this NEWS entry:

$ grep -R -A 1 rb_io_open_descriptor 
doc/NEWS/NEWS-3.3.0.md:  * `rb_io_open_descriptor()` is introduced to make an IO object from a file
doc/NEWS/NEWS-3.3.0.md-    descriptor.

Anyway, do you think something like this could be suitable fix?

diff --git a/ext/native/posix_serialport_impl.c b/ext/native/posix_serialport_impl.c
index 2f9325e..1afba51 100644
--- a/ext/native/posix_serialport_impl.c
+++ b/ext/native/posix_serialport_impl.c
@@ -80,12 +80,6 @@ int get_fd_helper(obj)
 VALUE sp_create_impl(class, _port)
    VALUE class, _port;
 {
-#ifdef HAVE_RUBY_IO_H
-   rb_io_t *fp;
-#else
-   OpenFile *fp;
-#endif
-
    int fd;
    int num_port;
    char *port;
@@ -109,10 +103,6 @@ VALUE sp_create_impl(class, _port)
    };
    struct termios params;
 
-   NEWOBJ(sp, struct RFile);
-   OBJSETUP((VALUE) sp, class, T_FILE);
-   MakeOpenFile((VALUE) sp, fp);
-
    switch(TYPE(_port))
    {
       case T_FIXNUM:
@@ -166,12 +156,7 @@ VALUE sp_create_impl(class, _port)
       rb_sys_fail(sTcsetattr);
    }
 
-#ifdef HAVE_RUBY_IO_H
-   fp->fd = fd;
-#else
-   fp->f = rb_fdopen(fd, "r+");
-#endif
-   fp->mode = FMODE_READWRITE | FMODE_SYNC;
+   VALUE sp = rb_io_open_descriptor(class, fd, FMODE_READWRITE | FMODE_SYNC, rb_str_new(port, 1), Qnil, NULL);
 
    return (VALUE) sp;
 }

[peterzhu2118 (Peter Zhu)]

That does not have too long history, seeing this NEWS entry:

You can implement a polyfill such as this one in io-console.

Anyway, do you think something like this could be suitable fix?

I think that looks correct, but I don't think you need to set the pathv so you don't need to create that string in he 4th argument.