Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#6393 closed defect (fixed)

[patch][cla] Existing setTimeout implementation overwritten by hostenv_rhino.js

Reported by: guest Owned by: dylan
Priority: high Milestone: 1.3
Component: TestFramework Version: 1.1.0
Keywords: Cc: jordi@…
Blocked By: Blocking:

Description

When running within Rhino, the hostenv_rhino.js file defines implementations of setTimeout and clearTimeout. However, the code doesn't check if those functions have already been defined. So those implementations will shadow prior definitions.

Other frameworks used in Rhino may define their own implementation of setTimeout, clearTimeout, setInterval, etc. It would be great if Dojo did not overwrite any existing implementation.

Examples of other implementations of setTimeout for Rhino are:

The main difference being whether they use Java threads or an event pump to behave more like the browsers which are single threaded.

  • Jordi Albornoz Mulligan

jordi@…

Attachments (1)

ticket6393.patch (1.7 KB) - added by guest 11 years ago.
Suggested patch - CLA submitted and acknowledged.

Download all attachments as: .zip

Change History (12)

Changed 11 years ago by guest

Attachment: ticket6393.patch added

Suggested patch - CLA submitted and acknowledged.

comment:1 Changed 11 years ago by guest

Patch was attached by Jordi Albornoz Mulligan <jordi@…>.

comment:2 Changed 11 years ago by dylan

Milestone: 1.2
Owner: changed from alex to dylan
Status: newassigned

comment:3 Changed 11 years ago by dylan

Summary: Existing setTimeout implementation overwritten by hostenv_rhino.js[patch][cla] Existing setTimeout implementation overwritten by hostenv_rhino.js

comment:4 Changed 11 years ago by dylan

Resolution: fixed
Status: assignedclosed

(In [14299]) fixes #6393, prevent rhino from overriding any existing [set|clear]Timeout implementations, thanks Jordi Albornoz Mulligan for the patch, \!strict

comment:5 Changed 11 years ago by Mark Wubben

Resolution: fixed
Status: closedreopened

In my current setup, resolving the setTimeout and clearTimeout variables does not throw an exception. Running Rhino 1.7.

I can't quite figure out why this is the case, but it wouldn't hurt adding:

	if(typeof(setTimeout) == 'undefined' || typeof(clearTimeout) == 'undefined'){ throw new Error; }

at the end of the try{} block.

comment:6 Changed 11 years ago by Mark Wubben

In fact, this could replace the existing check, since the variables are referenced anyway.

comment:7 Changed 11 years ago by Adam Peller

Milestone: 1.21.3

comment:8 Changed 11 years ago by dylan

Mark,

You mean like so:

Index: hostenv_rhino.js
===================================================================
--- hostenv_rhino.js    (revision 15698)
+++ hostenv_rhino.js    (working copy)
@@ -223,6 +223,7 @@
                                }
                                try{
                                        func();
+                                       if(typeof(setTimeout) == 'undefined' || typeof(clearTimeout) == 'undefined'){ throw new Error; }
                                }catch(e){
                                        console.debug("Error running setTimeout thread:" + e);
                                }

comment:9 Changed 11 years ago by Mark Wubben

Nope, meant the first block, to check for existing implementations:

try{
	setTimeout;
	clearTimeout;
}catch(e){

However, if I run this directly in a Rhino 1.7 shell (release 2, 11/05/2008) referencing setTimeout does throw an error. Don't quite recall what was happening earlier.

comment:10 Changed 11 years ago by dylan

Resolution: fixed
Status: reopenedclosed

Ok, marking as fixed then for now, as I can't reproduce either.

comment:11 Changed 11 years ago by Mark Wubben

We found a reproduction by running util/doh/runner.sh. Peller fixed the detection in r16312, see #8159.

Note: See TracTickets for help on using tickets.