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 728301 - Stop accessing object properties from worker thread
Stop accessing object properties from worker thread
Status: RESOLVED FIXED
Product: gom
Classification: Other
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Gom Maintainers
Gom Maintainers
Depends on: 700347
Blocks:
 
 
Reported: 2014-04-15 22:51 UTC by Bastien Nocera
Modified: 2014-11-05 17:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test.js (967 bytes, text/plain)
2014-04-15 22:51 UTC, Bastien Nocera
  Details
gom: Allow the empty string as an ID (1.08 KB, patch)
2014-09-26 19:03 UTC, Bastien Nocera
committed Details | Review
gom: Don't assert when the primary key isn't set (992 bytes, patch)
2014-09-26 19:03 UTC, Bastien Nocera
committed Details | Review
XXX Hack to detect property changes in != main thread (7.20 KB, patch)
2014-11-04 18:41 UTC, Bastien Nocera
rejected Details | Review
gom: Don't do get/set properties in != main thread (6.32 KB, patch)
2014-11-04 18:42 UTC, Bastien Nocera
committed Details | Review
XXX Hack to detect property changes in != main thread (7.88 KB, patch)
2014-11-05 17:24 UTC, Bastien Nocera
rejected Details | Review
gom: Don't do get/set properties in != main thread when reading (6.02 KB, patch)
2014-11-05 17:24 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2014-04-15 22:51:48 UTC
Created attachment 274404 [details]
test.js

Using gom master [1], I cannot run the class function to set the primary key on GomResourceClass

This is the equivalent C code:
https://git.gnome.org/browse/gom/tree/tests/test-gom-find.c#n122

The output for:
print (Item);
Item.set_table('items');
is:
$ ./tests/test.js
[object GObjectClass for Item]
(gjs:31329): Gjs-WARNING **: JS ERROR: TypeError: Item.set_table is not a function
@./tests/test.js:30


The full JavaScript code is attached.

[1]: http://git.gnome.org/browse/gom
Comment 1 Giovanni Campagna 2014-04-16 07:23:26 UTC
Ok, so what you want is Gom.Resource.set_table (class/static methods are not inherited in js), except that gjs does not expose those currently.

Thanks for the bug report. This particular bug has already been reported into our bug tracking system, but please feel free to report any further bugs you find.

*** This bug has been marked as a duplicate of bug 700347 ***
Comment 2 Bastien Nocera 2014-09-26 18:30:51 UTC
Reopening this and moving to gom, as gom will also need a bit of work, on top of the work done in bug 700347.
Comment 3 Bastien Nocera 2014-09-26 19:03:41 UTC
Created attachment 287202 [details] [review]
gom: Allow the empty string as an ID

gjs doesn't allow null values for string properties, so work-around that
by allowing '0' as a value.
Comment 4 Bastien Nocera 2014-09-26 19:03:46 UTC
Created attachment 287203 [details] [review]
gom: Don't assert when the primary key isn't set

gjs will want to set the primary key after the object has been
constructed. We'll assert() at the first use of the resource instead.
Comment 5 Bastien Nocera 2014-09-26 19:05:58 UTC
Attachment 287202 [details] pushed as d2e01ed - gom: Allow the empty string as an ID
Attachment 287203 [details] pushed as dc6164d - gom: Don't assert when the primary key isn't set
Comment 6 Christian Hergert 2014-09-26 23:33:11 UTC
Should we add a g_warning() when pspec is NULL?
Comment 7 Bastien Nocera 2014-09-26 23:53:52 UTC
(In reply to comment #6)
> Should we add a g_warning() when pspec is NULL?

No, it would produce warnings in valid Javascript code. Currently hunting down the last bug deep down in gjs to get saving working...
Comment 8 Bastien Nocera 2014-09-30 12:33:18 UTC
As per bug 737607, we can't access the object's properties from another thread. We should instead create the GomCommandBuilder commands in the main thread, populating it with values, and defer setting the new ID (if necessary) to the finished callback.

If performance turns out to be a problem, we'll make the current insert calls non-introspectable and provide another function for bindings to use.

We should also verify that no g_object_get/set_property() are being called from the worker thread as that would likely break Javascript bindings, Python bindings, etc.
Comment 9 Bastien Nocera 2014-11-04 18:41:56 UTC
Created attachment 289986 [details] [review]
XXX Hack to detect property changes in != main thread
Comment 10 Bastien Nocera 2014-11-04 18:42:12 UTC
Created attachment 289987 [details] [review]
gom: Don't do get/set properties in != main thread

Avoid manipulating properties from within a thread other than the
main thread. This breaks Javascript and Python bindings.
Comment 11 Bastien Nocera 2014-11-04 18:44:37 UTC
(In reply to comment #9)
> Created an attachment (id=289986) [details] [review]
> XXX Hack to detect property changes in != main thread

This is a hack to detect where get/set_property calls are getting made on a thread different from the main thread.

(In reply to comment #10)
> Created an attachment (id=289987) [details] [review]
> gom: Don't do get/set properties in != main thread
> 
> Avoid manipulating properties from within a thread other than the
> main thread. This breaks Javascript and Python bindings.

This only fixes Gom.Resource.save* functions.
Comment 12 Bastien Nocera 2014-11-04 18:57:23 UTC
Comment on attachment 289987 [details] [review]
gom: Don't do get/set properties in != main thread

Committed with a more precise commit message
Comment 13 Bastien Nocera 2014-11-05 17:24:11 UTC
Created attachment 290041 [details] [review]
XXX Hack to detect property changes in != main thread
Comment 14 Bastien Nocera 2014-11-05 17:24:17 UTC
Created attachment 290042 [details] [review]
gom: Don't do get/set properties in != main thread when reading

This is the 2nd part of b14e7044bf5cbfe4b6baac4b3ce66dba7dc8f458.

We now try to avoid creating, and setting GomResource items when reading
from the database. Instead, the items are created from cached results as
needed when gom_resource_group_get_index() is called.
Comment 15 Bastien Nocera 2014-11-05 17:26:15 UTC
Comment on attachment 290041 [details] [review]
XXX Hack to detect property changes in != main thread

This is the same hack, but the main_thread is set from within the class_init of the adapter as well, which means that it works with gjs.
Comment 16 Bastien Nocera 2014-11-05 17:36:54 UTC
Attachment 290042 [details] pushed as b43eb35 - gom: Don't do get/set properties in != main thread when reading

One problem left in gjs with instantiating Javascript GObjects from C. See bug 739678.