413671 - xulrunner component '@mozilla.org/thread-manager;1' crash or hangs on 1.9b3pre, while same code doesn't crash 1.9b2pre

Status: RESOLVED INVALID

(Toolkit Graveyard :: XULRunner, defect)

Description

[Roberto Ostinelli]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; it; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Build Identifier: xulrunner-1.9b3pre.en-US.win32.zip build 22-Jan-2008 11:46

Attached code is a simple xul application that uses a thread component '@mozilla.org/thread-manager;1'. To start code, call the startThread() function.

Running the attached code should normally only display an alert box with the text 'thread has successfully passed this content back.'.

This worked perfectly in 1.9b2pre, crashes or hangs xulrunner since 1.9b3pre.

R.

Reproducible: Always

Steps to Reproduce:
1. include attached code in a XULrunner project.
2. call the startThread() function.
Actual Results:  
Crash or hangs, no alert box is displayed.

Expected Results:  
Running the attached code should normally only display an alert box with the text 'thread has successfully passed this content back.'.

//code:


function startThread() {
	try {
		var input = 'thread has successfully passed this content back.';
		var execute = function(input) { return input; };
		var callback = function(result) { alert(result); };
		var myThread = new XULthread(execute,input,callback);
		myThread.start();
	} catch(e) { alert(e) };
}

var XULthread = function() { this.initialize.apply(this, arguments); };
var XULexecute = function() { this.initialize.apply(this, arguments); };
var XULcallback = function() { this.initialize.apply(this, arguments); };

XULthread.prototype = {
	_executeFunction: null,
	_callbackFunction: null,
	_input: null,
	initialize: function(executeFunction, input, callbackFunction) {
		this._executeFunction = executeFunction;
		this._input = input;
		this._callbackFunction = callbackFunction;
	},
	start: function () {
		background = Components.classes['@mozilla.org/thread-manager;1'].getService().newThread(0);
		main = Components.classes['@mozilla.org/thread-manager;1'].getService().mainThread;
		background.dispatch(new XULexecute(this._executeFunction, this._input, this._callbackFunction), 
			background.DISPATCH_NORMAL);
	}
}

XULexecute.prototype = {
	_executeFunction: null,
	_callbackFunction: null,
	_input: null,
	initialize: function(executeFunction, input, callbackFunction) {
		this._executeFunction = executeFunction;
		this._input = input;
		this._callbackFunction = callbackFunction;
	},
	run: function() {
		try {
			// variable that holds result of execution
			var result = this._executeFunction(this._input);
			// Callback to the main thread
			main.dispatch(new XULcallback(this._callbackFunction, result), background.DISPATCH_NORMAL);
		} catch (err) {
			Components.utils.reportError(err);
		}
	},
	QueryInterface: function(iid) {
		if (iid.equals(Components.interfaces.nsIRunnable) || iid.equals(Components.interfaces.nsISupports)) {
			return this;
		}
		throw Components.results.NS_ERROR_NO_INTERFACE;
	}
}

XULcallback.prototype = {
	_callbackFunction: null,
	_result: null,
	initialize: function(callbackFunction, result) {
		this._callbackFunction = callbackFunction;
		this._result = result;
	},
	run: function() {
		try {
			// This is where we can work with the main thread after the job
			this._callbackFunction(this._result);

		} catch (err) {
			Components.utils.reportError(err);
		}
	},
	QueryInterface: function(iid) {
		if (iid.equals(Components.interfaces.nsIRunnable) || iid.equals(Components.interfaces.nsISupports)) {
			return this;
		}
		throw Components.results.NS_ERROR_NO_INTERFACE;
	}
}

Comment 1

[Roberto Ostinelli]

Attachment: Code to reproduce the bug. This is the same code in available in the bug description, only added here for your convenience.

This is the same code reported in the bug, only zipped for your convenience.

Comment 2

[Benjamin Smedberg]

Roberto, do you have a stacktrace?

Comment 3

[timeless]

this testcase isn't legal.

you aren't allowed to use a dom global on any thread other than the main thread, and your testcase is clearly using alert() which means you have a dom global scope.

yes, we should prevent you from triggering that problem, but that should be the subject of some other bug (in fact, i hope i already filed that bug a while ago).

Comment 4

[Roberto Ostinelli]

(In reply to comment #2)
> Roberto, do you have a stacktrace?
> 
unfortunately not. the crashing/hanging is random and at first i didn't expect to file a bug.

Comment 5

[Roberto Ostinelli]

(In reply to comment #3)
> this testcase isn't legal.
> 
> you aren't allowed to use a dom global on any thread other than the main
> thread, and your testcase is clearly using alert() which means you have a dom
> global scope.

i am not sure i do understand. function startThread() is being called by a XUL window which is the main thread. alert() is therefore called by the main thread. the threading only takes place in the XULthread class... or am i missing something?

moreover.. as per description this works perfectly in 1.9b2pre.

r.

Comment 6

[timeless]

you may not use xul windows to create javascript objects which you then pass to other threads. the dom global object is not threadsafe, and all js that is created with that as a parent scope will have it as a parent scope, which results in objects which are not threadsafe. you will crash. it isn't legal <period>

i don't care if you think it might have worked, it was broken, it wasn't legal.

note: instantiating an xpcom object (no matter what the implementation language is, e.g. javascript) from within a dom scope is ok, because the object is wrapped and its internal global object is *not* the dom window, but instead an object that /might/ be threadsafe (and is in the case of JS).

Comment 7

[Roberto Ostinelli]

again, thanks for the input.

in this case, it is not clear to me how to use threading to release applications' GUI from freezing when performing long tasks, and passing back the results from the tasks [such as, for instance, time to completion or task advancement].

if you do have clues/alternative solutions to this matter, any sharing will be most welcome, notably to resolve this bug to invalid.

Comment 8

[timeless]

At a bare minimum, you need a component ("@capricorn/threaded-xulthread;1") declares nsIClassInfo.flags THREADSAFE. How exactly you choose to send message across threads is up to you (you can have your code on the other thread use the proxy object manager to send messages back, or you can simply push things around inside your javascript component and have the dom code poll your javascript component). Your dom code uses Components.classes["@capricorn/threaded-xulthread;1"].createInstance() (or a variant) and talks to the thread manager directly, passing the "@capricorn/threaded-xulthread;1" object.

The key is that "XULthread" from your example needs to be a real component (possibly written in js as a components/foo.js, not just a script that is hosted by dom), and it should not store DOM objects into it without getting proxies for them if it plans to use them on another thread - any proxies you get for them *must* be retrieved from the thread on which you received the objects [it's easiest to do this eagerly, i.e., when you're given the object:

Components.classes["@mozilla.org/xpcomproxy;1"].getService(Components.interfaces.nsIProxyObjectManager).getProxyForObject(null, Components.interfaces["nsIsomething"], unsafe_object, /* INVOKE_SYNC | FORCE_PROXY_CREATION */ 5)

] - alternatively you can have a threadsafe object which can proxy to itself on the right thread (i.e., the dom thread) and then use them (I've done both, it depends on how much you know about the objects you're going to poke, if you don't know much, it's easier to call yourself on the main thread and have your own object then poke the unsafe object).

Comment 9

[Roberto Ostinelli]

thank you timeless.

your explanations clarify pretty much. i have seen MANY examples using threading like i did on the internet, therefore hopefully this bug will help the persons experiencing my same issues to be pointed out in a right direction.

i therefore resolve this bug to invalid.

r.