After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 687987 - should not use guint-based GSource APIs
should not use guint-based GSource APIs
Status: RESOLVED OBSOLETE
Product: gjs
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2012-11-09 14:21 UTC by Dan Winship
Modified: 2018-01-27 11:51 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Dan Winship 2012-11-09 14:21:25 UTC
memory-managed bindings should use the GSource*-based GSource APIs, not the guint-based ones, since

  a) in some programs, the IDs may eventually overflow and get reused
     (bug 687098)

  b) g_source_remove() is O(n), whereas g_source_destroy() is O(1)

JS doesn't have a guaranteed-to-be-pointer-sized integer type, so to upgrade the existing APIs you'd have to make them return objects instead of integers, which could in theory break some users...
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-11-09 19:14:23 UTC
We wouldn't have to break API, because this would still work in the context of an object:

    this._fooTimeoutId = Mainloop.timeout_add(...);

    if (this._fooTimeoutId > 0) {
        Mainloop.source_remove(this._fooTimeoutId);
        this._fooTimeout = 0;
    }

All objects are > 0 (and null is not)!
Comment 2 Giovanni Campagna 2012-11-10 16:29:06 UTC
>>> ({a:5}) > 0
false

here...

Maybe we should introduce Mainloop.idle_create() and Mainloop.timeout_create().
Btw, the problem with that is that GLib.Source.set_callback() is good enough for idle and timeout, but not for GChildWatch for example. (This is under the assumption that eventually we want to turn Mainloop into JS sugar above GLib)
Comment 3 Dan Winship 2012-11-15 13:57:05 UTC
(In reply to comment #2)
> Btw, the problem with that is that GLib.Source.set_callback() is good enough
> for idle and timeout, but not for GChildWatch for example.

g_source_set_closure() (in libgobject) exists to deal with this problem.
Comment 4 Giovanni Campagna 2012-11-15 15:05:11 UTC
And that's even worse, because not all GSources have a marshaller. In particular, GChildWatch doesn't.
Comment 5 Dan Winship 2012-11-15 15:40:29 UTC
Hm. That's a bug then.
Comment 6 Giovanni Campagna 2013-01-19 00:00:58 UTC
Due to a short commit message and a fast review, Mainloop in gjs master has gained the following convenience methods:

idle_source(handler)
timeout_source(timeout, handler)
timeout_seconds_source(timeout, handler)

They all return a new GSource, ready to be attached to some GMainContext.
Indeed, idle_add is just idle_source(handler).attach(null);

Now the question is, should we deprecate {idle,timeout}_add()? Should we port existing users to the GSource based API?
Should we move away from using GLib API directly?
Comment 7 GNOME Infrastructure Team 2018-01-27 11:51:36 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gjs/issues/64.