create3000 / cobweb

Cobweb is now X_ITE
https://github.com/create3000/x_ite
Other
10 stars 5 forks source link

event loops not being broken. #43

Closed splace closed 6 years ago

splace commented 6 years ago

when investigating several worlds that 'lock-up', it seems that the same event, at the same time-stamp, is not being blocked.

(this was actually found in titania, but i assume cobweb is the underlying code.)

andreasplesch commented 6 years ago

I think it would be best to come up with a minimal x3d which exhibts the issue and file issues both under titania and x_ite.

splace commented 6 years ago

tracked down the bug, confirmed it, tried to find a conformance test and reported.

i think the developer should just 'know' if this was the case, saving me much time.

sometimes developers, seem to me, to under-appreciate the time/effort needed to put in comprehensible bug report, (after having been frustrated from doing what you were by the bug.) more than half of mine are never responded to, and more than half of the remainder get the response, 'bug report not good enough', when i know a little effort is all that is needed, effort that is massively less for them than for me.

create3000 commented 6 years ago

Could you please provide your code for 'look-up' a world and can you describe in what case the event is not blocked.

I appreciate every bug report because I know that one could spend much time to come to it but I as a developer must reproduce the bug again and again to find the bug. This is sometimes a huge investigation but then I can find the line that causes the bug and fix it. For that it is very helpful to have as much information about what causes the bug. This can be very obviously information or also not so obviously information. Thank you very much for your response.

create3000 commented 6 years ago

Please note for the future to use X_ITE instead of Cobweb and to report but at X_ITE:issues. Thanks.

splace commented 6 years ago

OK, so event loop breaking should work. shame i couldn't find a conformance test for this.

from things like this; http://realism.com/blog/x3d-event-model, it seemed that it might be something that was being thought of as optional, or going that way, particularly in web (DOM) based rendering.

i will attempt to cut down the broken content to get a minimum example.

splace commented 6 years ago

the min test case, causes a crash (see log).

test2.wrl

notice: remarking out the event loop causing line, in the JS, prevents the crash, so possibly related.

console out:

flatpak run de.create3000.titania

*** The browser is requested to replace the world with ''.

*** The browser is requested to replace the world with ''.

*** The browser is requested to replace the world with ''.

Done loading scene 'file:///app/share/titania/tools/library/RoundedRectangle2D.x3dv'. Done loading image 'file:///app/share/titania/tools/library/gradient.png'. Done loading scene 'file:///app/share/titania/tools/library/RoundedRectangle2D.x3dv'. Done loading image 'file:///app/share/titania/tools/library/gradient.png'.



Welcome to Titania X3D Browser 4.3.1: Compiled at Aug 28 2018 05:41:07 Current Graphics Renderer Name: X.Org AMD Radeon HD 8600 Series (AMD OLAND / DRM 3.25.0 / 4.17.17-87.current, LLVM 5.0.0) OpenGL extension version: 3.0 Mesa 17.2.7 Shading language version: 1.30, 1.0 es X_ITE (default) Rendering Properties Texture units: 8 / 184 Max texture size: 16384 × 16384 pixel Max lights: 8 Max clip planes: 6 Antialiased: false Color depth: 32 bits Texture memory: 1.0 kB Max vertex uniform vectors: 4096 Max fragment uniform vectors: 4096 Max vertex attribs: 16 Current Javascript Engine Name: Mozilla Foundation SpiderMonkey Description: JavaScript-C 1.8.5 2011-03-31 Version: ECMAv5



Done loading scene 'file:///app/share/titania/tools/AngleGridTool.x3dv'. Done loading scene 'file:///app/share/titania/tools/GridTool.x3dv'. Done loading scene 'file:///app/share/titania/tools/AxonometricGridTool.x3dv'. Done loading scene '/app/share/titania/ui/Logo.x3dv'.



Welcome to Titania X3D Browser 4.3.1: Compiled at Aug 28 2018 05:41:07 Current Graphics Renderer Name: X.Org AMD Radeon HD 8600 Series (AMD OLAND / DRM 3.25.0 / 4.17.17-87.current, LLVM 5.0.0) OpenGL extension version: 3.0 Mesa 17.2.7 Shading language version: 1.30, 1.0 es X_ITE (default) Rendering Properties Texture units: 8 / 184 Max texture size: 16384 × 16384 pixel Max lights: 8 Max clip planes: 6 Antialiased: false Color depth: 32 bits Texture memory: 1.0 kB Max vertex uniform vectors: 4096 Max fragment uniform vectors: 4096 Max vertex attribs: 16 Current Javascript Engine Name: Mozilla Foundation SpiderMonkey Description: JavaScript-C 1.8.5 2011-03-31 Version: ECMAv5



Done loading scene 'file:///app/share/titania/pages/about/splash.x3dv'.



Welcome to Titania X3D Browser 4.3.1: Compiled at Aug 28 2018 05:41:07 Current Graphics Renderer Name: X.Org AMD Radeon HD 8600 Series (AMD OLAND / DRM 3.25.0 / 4.17.17-87.current, LLVM 5.0.0) OpenGL extension version: 3.0 Mesa 17.2.7 Shading language version: 1.30, 1.0 es X_ITE (default) Rendering Properties Texture units: 8 / 184 Max texture size: 16384 × 16384 pixel Max lights: 8 Max clip planes: 6 Antialiased: false Color depth: 32 bits Texture memory: 1.0 kB Max vertex uniform vectors: 4096 Max fragment uniform vectors: 4096 Max vertex attribs: 16 Current Javascript Engine Name: Mozilla Foundation SpiderMonkey Description: JavaScript-C 1.8.5 2011-03-31 Version: ECMAv5



Done loading scene 'file:///app/share/titania/pages/about/splash.x3dv'. Done loading scene '/app/share/titania/pages/about/library/Colors.x3dv#Red'. Done loading scene 'file:///app/share/titania/tools/library/ToolShader.x3dv'. Done loading scene 'file:///app/share/titania/tools/library/ToolShader.x3dv'. Done loading scene 'file:///app/share/titania/tools/library/ToolShader.x3dv'. Done loading scene 'file:///app/share/titania/tools/library/Grid.x3dv#Grid'. Done loading scene 'file:///app/share/titania/tools/library/AxonometricGrid.x3dv#AxonometricGrid'. Done loading scene 'file:///app/share/titania/tools/library/AngleGrid.x3dv#AngleGrid'.

*** The browser is requested to replace the world with 'file:///app/share/titania/pages/about/splash.x3dv'.

*** The browser is requested to replace the world with 'file:///app/share/titania/pages/about/splash.x3dv'.

*** The browser is requested to replace the world with '/app/share/titania/ui/Logo.x3dv'.

Done loading scene 'file:///app/share/titania/tools/library/RoundedRectangle2D.x3dv'. Done loading image 'file:///app/share/titania/tools/library/gradient.png'. Done loading scene 'file:///home/simon/Desktop/monitor.x3dv'. Done loading scene 'file:///home/simon/Desktop/test2.wrl'. Done loading scene 'file:///app/share/titania/tools/library/RoundedRectangle2D.x3dv'. Done loading image 'file:///app/share/titania/tools/library/gradient.png'. Done loading scene 'file:///app/share/titania/tools/library/RoundedRectangle2D.x3dv'. Done loading image 'file:///app/share/titania/tools/library/gradient.png'.

*** The browser is requested to replace the world with 'file:///home/simon/Desktop/monitor.x3dv'.

*** The browser is requested to replace the world with 'file:///home/simon/Desktop/test2.wrl'.

(titania:3): Gtk-CRITICAL **: _gtk_accel_group_attach: assertion 'g_slist_find (accel_group->priv->acceleratables, object) == NULL' failed ################################################################################ #

Backtrace

#

Error: signal 11 SIGSEGV

#

Invalid memory reference

# ################################################################################ /app/lib/libtitania-standard.so.0(_ZN7titania17backtrace_symbolsB5cxx11Em+0x2b) [0x7f0433e5d70b] /app/lib/libtitania-standard.so.0(+0x110d2f) [0x7f0433e5dd2f] /app/lib/libtitania-standard.so.0(+0x111406) [0x7f0433e5e406] /lib/libc.so.6() [0x3153a330b0] /app/lib/libmozjs185.so.1.0(+0xb1b3c) [0x7f0439829b3c] /app/lib/libmozjs185.so.1.0(+0xb1e6f) [0x7f0439829e6f] /app/lib/libmozjs185.so.1.0(+0xdba5f) [0x7f0439853a5f] /app/lib/libmozjs185.so.1.0(+0xdbebd) [0x7f0439853ebd] /app/lib/libmozjs185.so.1.0(JS_CallFunctionValue+0x42) [0x7f04397c22e2] /app/lib/libtitania-x3d.so.0(_ZN7titania3X3D12spidermonkey7Context9set_fieldEPNS0_18X3DFieldDefinitionERKm+0x8d) [0x7f0436467eed] /app/lib/libtitania-x3d.so.0(_ZNK7titania3X3D9X3DOutput16processInterestsEv+0xbd) [0x7f0435ccff8d] /app/lib/libtitania-x3d.so.0(_ZN7titania3X3D18X3DFieldDefinition12processEventERKSt10shared_ptrINS0_5EventEE+0xbf) [0x7f0435cf6b1f] /app/lib/libtitania-x3d.so.0(_ZN7titania3X3D6Router13processEventsEv+0x39) [0x7f043665d979] /app/lib/libtitania-x3d.so.0(_ZN7titania3X3D17X3DBrowserContext6updateEv+0x168) [0x7f0435e688c8] /app/lib/libtitania-x3d.so.0(_ZN7titania3X3D17X3DBrowserContext9on_renderEv+0x11) [0x7f0435e69391] /app/lib/libtitania-x3d.so.0(_ZN7titania3X3D19X3DRenderingSurface10on_timeoutEv+0x1a5) [0x7f0436656cf5] /app/lib/libglibmm-2.4.so.1(+0x57b92) [0x7f0438fc3b92] /lib/libglib-2.0.so.0(+0x4ad53) [0x7f04377a9d53] /lib/libglib-2.0.so.0(g_main_context_dispatch+0x15a) [0x7f04377a92ca] /lib/libglib-2.0.so.0(+0x4a688) [0x7f04377a9688] /lib/libglib-2.0.so.0(g_main_context_iteration+0x2c) [0x7f04377a973c] /lib/libgio-2.0.so.0(g_application_run+0x20d) [0x7f043b69c9ad] titania(_ZN7titania4puck18BrowserApplication4mainEiPPc+0x2f) [0x78ceaf] titania(main+0x19d) [0x78d6dd] /lib/libc.so.6(__libc_start_main+0xf1) [0x3153a20291] titania(_start+0x2a) [0x7bc89a]

andreasplesch commented 6 years ago

copy at titania: https://github.com/create3000/titania/issues/26

x_ite did not complain for me.

splace commented 6 years ago

this now causes the lock-up as described.

test3.wrl

x_ite did not complain for me.

so what did it do with that broken file?

andreasplesch commented 6 years ago

For easier reading, test3.wrl:

#VRML V2.0 utf8

DEF integerPosition Script {
    eventIn SFVec3f realPosition
    eventOut SFVec3f    realPositionOut
    url "javascript: 
    function realPosition(value, timestamp) { 
            realPositionOut=value;
                        Browser.print(timestamp);
        }
    "}

Group {
    children[
        DEF sensor ProximitySensor{size 100 100 100}
    ]
    ROUTE sensor.position_changed TO integerPosition.realPosition
    ROUTE integerPosition.realPositionOut TO integerPosition.realPosition
}

I added the timestamp logging. This looping does lock up x_ite for me as well. CPU is up at 100% and there is an infinite loop into the script. The timestamp does not advance, the event loop is not broken.

andreasplesch commented 6 years ago

Here is the loop breaking rule:

https://www.web3d.org/files/specifications/19775-1/V3.3/Part01/concepts.html#Loops

andreasplesch commented 6 years ago

Here is how loop breaking could be done in the script until the browser can do it:

DEF integerPosition Script {
    eventIn SFVec3f realPosition
    eventOut SFVec3f    realPositionOut
    url "javascript: 
    var prev=0;
    function realPosition(value, ts) {
        if (ts > prev) {
            Browser.print(ts);
            realPositionOut=value;
            prev = ts;}
        }
    "}
create3000 commented 6 years ago

Commented your test2.wrl, because I found some logical errors in handling javascript variables.

01 #VRML V2.0 utf8
02
03 DEF integerPosition Script {
04  eventIn SFVec3f realPosition
05  url "javascript: 
    // at this level the variable 'realPosition' contains the function object defined below
06  function realPosition(value) { 
    // at this level the variable 'realPosition' contains the function object defined above
07  realPosition=value;
    // now  the variable 'realPostion' has a new content 'value'
08 }
09 "
10  }
11
12 Group {
13  children[
14   DEF sensor ProximitySensor{size 100 100 100}
15 ]
16  ROUTE sensor.position_changed TO integerPosition.realPosition
17  ROUTE integerPosition.realPositionOut TO integerPosition.realPosition
18 }

In line 07 you cannot do and assignment to realPosition to send events because the variable realPosition contains the function object 'realPosition' defined in line 06. You only could do a function call realPosition (someValue) but then you are self responsible for handling the recursive call.

It is not possible neither in Titania nor X_ITE to assign to an inputOnly (eventIn) if a callback function with the same name is defined within the script. You only can call the callback function directly.

create3000 commented 6 years ago

Defining a function in JavaScript will define a variable with the same name:

function foo () { }

is almost the same as

var foo = function () {}

create3000 commented 6 years ago

if you want to assign to an inputOnly of a Script node you can do:

DEF integerPosition Script {
  eventIn  SFVec3f  realPosition
  url   "javascript: 
function realPosition(value) { 
  var self = Browser .currentScene .getNamedNode ('integerPosition');
  self .realPosition = value;
}
"
}

But then you are self responsible for handling the recusive call (event loop).

create3000 commented 6 years ago

Looking at the modified Script from Andreas:

#VRML V2.0 utf8

DEF integerPosition Script {
    eventIn SFVec3f realPosition
    eventOut SFVec3f    realPositionOut
    url "javascript: 
    function realPosition(value, timestamp) { 
            realPositionOut=value;
                        Browser.print(timestamp);
        }
    "}

Group {
    children[
        DEF sensor ProximitySensor{size 100 100 100}
    ]
    ROUTE sensor.position_changed TO integerPosition.realPosition
    ROUTE integerPosition.realPositionOut TO integerPosition.realPosition
}

This will indeed cause a infinite loop because the event loop detection is always broken if there is a Script within the loop. The Script Author is here responsible to break the loop just as well as he is to break recursive function calls in JavaScript.

Have a look at Recursion in Functional JavaScript — SitePoint: https://www.sitepoint.com/recursion-functional-javascript/

andreasplesch commented 6 years ago

Could this rule be implemented at the ROUTE level ?

If a field is connected to another field via a ROUTE, an implementation shall send only one event per ROUTE per timestamp. This also applies to scripts..

create3000 commented 6 years ago

Commited a fix for X_ITE: https://github.com/create3000/x_ite/commit/53a8cbb1b229ece027dc307a66938aa9c93a02ba

Tested with test3.wrl

create3000 commented 6 years ago

Fixed event loop breaking when script is involved in Titania. https://github.com/create3000/titania/commit/46b0b1424a5facd9022ba97ade2cec3d399365bb

create3000 commented 6 years ago

X_ITE urls for testing are: https://code.create3000.de/x_ite/alpha/dist/x_ite.css https://code.create3000.de/x_ite/alpha/dist/x_ite.js

andreasplesch commented 6 years ago

There are some spurious characters at line 13963 of https://code.create3000.de/x_ite/alpha/dist/x_ite.js

...

define ('x_ite/Base/Events',[],function ()

and line 24623 and 50071, 64409, 65161, 65279, 70232

?

andreasplesch commented 6 years ago

After removing the strange, extra characters, I can confirm that the fix works for breaking event loops in x_ite.

splace commented 6 years ago

also for me this works.

(unfortunately test2 still crashes, since it now seems to be unrelated, i will make a new issue on x-ite)