Crash when calling the mozIStorageBindingParams methods from a working thread

NEW
Unassigned

Status

()

Core
XPConnect
--
critical
8 years ago
10 months ago

People

(Reporter: Adrià Mercader, Unassigned)

Tracking

({dev-doc-needed})

Trunk
x86_64
All
dev-doc-needed
Points:
---

Firefox Tracking Flags

(blocking2.0 -)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre
Build Identifier: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre

When calling a method of mozIStorageBindingParams like bindByName from outside the main thred, the application crashes and exits. If called from the main thread, the code is executed successfully. Also, binding the parameter via mozIStorageStatementParams (stmt.params.xxxx = yyyy) inside a working thread works fine.

Reproducible: Always

Steps to Reproduce:
I've attached a file reproducing the following steps:

1.Create a function that:
a.Opens a storage connection and creates a statement with a binding
b.Creates a new mozIStorageBindingParamsArray object and a mozIStorageBindingParams object and binds the parameter with the bindByName() function.
c. Executes the statement async
2.Run the previous function in a separate thread
Actual Results:  
The application crashes when calling the bindByName() method.

Expected Results:  
Bind the parameter and follow with the statement execution.

The code works on a working thread in Firefox 3.6.8.

The shell console shows the lines included in the file console.txt
(Reporter)

Comment 1

8 years ago
Created attachment 464775 [details]
Test case to reproduce the crash (run test_thread_DB())
(Reporter)

Comment 2

8 years ago
Created attachment 464776 [details]
Database to use with the test
(Reporter)

Comment 3

8 years ago
Created attachment 464777 [details]
Console output after crash

Updated

8 years ago
Attachment #464775 - Attachment mime type: application/javascript → text/plain

Comment 4

8 years ago
Have you got a stack trace or crash report ID?
(Reporter)

Comment 5

8 years ago
Created attachment 464819 [details]
Stack trace
(Reporter)

Comment 6

8 years ago
(In reply to comment #4)
> Have you got a stack trace or crash report ID?

I've attached a stack trace created following this instructions:

http://kb.mozillazine.org/Getting_a_stacktrace_with_gdb

Please let me know if this is not what you asked me.
(In reply to comment #0)
> When calling a method of mozIStorageBindingParams like bindByName from outside
> the main thred, the application crashes and exits. If called from the main
> thread, the code is executed successfully. Also, binding the parameter via
> mozIStorageStatementParams (stmt.params.xxxx = yyyy) inside a working thread
> works fine.
You should not be using the stmt.params.xxxx methods on a different thread because it uses XPConnect, which AFAIK is main thread only.

> 1.Create a function that:
> a.Opens a storage connection and creates a statement with a binding
> b.Creates a new mozIStorageBindingParamsArray object and a
> mozIStorageBindingParams object and binds the parameter with the bindByName()
> function.
> c. Executes the statement async
> 2.Run the previous function in a separate thread
> Actual Results:  
None of this should be illegal per the code.

So, a few things concern me:
1) is this code being run from a window, or a JS component?
2) The error log you attach says there are aborts from the cycle collector being used off of the main thread.  Storage is not hooked up to the cycle collector, so it isn't the cause of those.
XPCVariants are cycle collected, and the exploding bind call uses them...
hurg.  We may need to change that to the new jsval for idls then :/

Still need my question to (1) answered though.

Comment 10

8 years ago
xpconnect was *not* supposed to be main thread only. it historically did support threaded stuff. your code should recognize it's not on the main thread and either throw an exception or return some other object.
(In reply to comment #10)
> xpconnect was *not* supposed to be main thread only. it historically did
> support threaded stuff. your code should recognize it's not on the main thread
> and either throw an exception or return some other object.
Alright, maybe my assumption was wrong.  Still need the answer to (1) since if it's running from a window, that isn't threadsafe (and is cycle collected).

Comment 12

8 years ago
sure.

Comment 13

8 years ago
reporter: sorry, the stack trace you provided is useless.

https://developer.mozilla.org/en/How_to_get_a_stacktrace_for_a_bug_report#Linux

Please note that if you are using a distro build you need to install their symbols. If you're using a 32bit build we provide, crash-reporter should work. if you're using your own build, you need --disable-strip and --enable-debugger-info-modules/--enable-debug (preferably the latter since it'll complain sooner).
(Reporter)

Comment 14

8 years ago
(In reply to comment #13)
> reporter: sorry, the stack trace you provided is useless.
> 
> https://developer.mozilla.org/en/How_to_get_a_stacktrace_for_a_bug_report#Linux
> 
> Please note that if you are using a distro build you need to install their
> symbols. If you're using a 32bit build we provide, crash-reporter should work.
> if you're using your own build, you need --disable-strip and
> --enable-debugger-info-modules/--enable-debug (preferably the latter since
> it'll complain sooner).


I couldn't get a stacktrace yet following those instructions, I will keep trying. I could get the crash ID though:

bp-4bbc9627-acf8-48f1-999f-ebd9e2100811

As for question (1), I'm executing it from a javascript file loaded from a Firefox extension, or also in the Javascript shell of the Extension Developer extension, but it is not part of a XPCOM component, if that was what you were asking for.
(In reply to comment #14)
> As for question (1), I'm executing it from a javascript file loaded from a
> Firefox extension, or also in the Javascript shell of the Extension Developer
> extension, but it is not part of a XPCOM component, if that was what you were
> asking for.
This means your global object is a window, which is no good.  If you want to use multithreaded code, you have to do it from a JS component.
(Reporter)

Comment 16

8 years ago
(In reply to comment #15)

> This means your global object is a window, which is no good.  If you want to
> use multithreaded code, you have to do it from a JS component.

Uops. Can you point me to some resource explaining how to use threads inside XPCOM components?

Does this mean that this is not a bug? In Firefox 3.6.8 works fine for me.
(In reply to comment #16)
> Uops. Can you point me to some resource explaining how to use threads inside
> XPCOM components?
I don't know if we have any documentation that explains how to make threads inside a component.  It's not different from how you are doing it now; it's just in a JS component.

> Does this mean that this is not a bug? In Firefox 3.6.8 works fine for me.
Seems like this is not a bug and is probably a result of us making the cycle collector error harder when someone is using it off of the main thread.
Keywords: dev-doc-needed
(Reporter)

Comment 18

8 years ago
I've run the code inside a JS component and calling bindByName still crashes Firefox when executed from a working Thread, but just in 4.0b4pre, in 3.6.8 works fine.

Crash ID:
127ff231-0f89-5973-32a61960-6799ffd3

Could you please have a look at testcomponent.js and tell me if I haven't done any mistake? It's the first component I've created.

To sum up:
* Calling TestComponent.test_DB() works fine in FF 3.6.8 and 4.0b4pre
* Calling TestComponent.test_thread_DB() works fine in FF 3.6.8 but crashes 4.0b4pre
(Reporter)

Comment 19

8 years ago
Created attachment 465611 [details]
Test component to reproduce the crash (run test_thread_DB())
Attachment #464775 - Attachment is obsolete: true
(Reporter)

Comment 20

8 years ago
Created attachment 465660 [details]
Stack trace (2nd try)

I've managed to get a stack trace of the crash with some more info (still has some warnings though)
Attachment #464819 - Attachment is obsolete: true
This feels like it might be a duplicate of bug 464678, but something has clearly changed on trunk that makes us crash now when we didn't in 3.6.  Looks to be a bug in XPConnect.
Status: UNCONFIRMED → NEW
Component: Storage → XPConnect
Depends on: 464678
Ever confirmed: true
Product: Toolkit → Core
QA Contact: storage → xpconnect
Version: unspecified → Trunk
(Reporter)

Comment 22

8 years ago
Sorry, I'm a little confused and I would like to clarify this. Is this a bug because:

1- In theory, you can bind parameters to a statement inside a working thread, but right now this is crashing.
or
2- You are not allowed to bind parameters to a statement inside a working thread, but it should throw an exception instead of crashing?

Thanks
It's (1).  We might be able to work around this by using jsvals instead of nsIVariants...
blocking2.0: --- → ?

Comment 24

8 years ago
We changed the cycle collector to crash (intentionally) if you try to use a cycle collected object on the wrong thread, instead of merely faulting.

Yes, you should use jsval. It's not clear that this needs to block, though. This would only happen with extensions, right?
(In reply to comment #24)
> Yes, you should use jsval. It's not clear that this needs to block, though.
> This would only happen with extensions, right?
Yes.  Not fixing this means it's difficult (but still possible) to bind by name instead of using bind by index.
(Reporter)

Comment 26

8 years ago
(In reply to comment #25)
> Yes.  Not fixing this means it's difficult (but still possible) to bind by name
> instead of using bind by index.

Just for record, using bindByIndex instead of bindByName also makes Firefox crash.
(In reply to comment #26)
> Just for record, using bindByIndex instead of bindByName also makes Firefox
> crash.
Right because they both use nsIVariants.  You'd have to use one of these:
http://mxr.mozilla.org/mozilla-central/source/storage/public/mozIStorageBaseStatement.idl#77

Of course, those are deprecated and I was planning on removing them after Firefox 4.0...

Comment 28

8 years ago
Didn't I have a patch to remove that callers of that stuff, which no one had reviewed? :(
(In reply to comment #28)
> Didn't I have a patch to remove that callers of that stuff, which no one had
> reviewed? :(
In various places, yes.  It's still in my long review queue.

Updated

8 years ago
blocking2.0: ? → -
You need to log in before you can comment on or make changes to this bug.