596970 - ASSERTION: You can't dereference a NULL nsCOMPtr with operator->() in AsyncStatement::~AsyncStatement()

Status: RESOLVED FIXED

(Core :: SQLite and Embedded Database Bindings, defect)

Description

[Marco Bonardo [:mak]]

Connection *forgottenConn = nsnull;
    mDBConnection.swap(forgottenConn);
    (void)::NS_ProxyRelease(mDBConnection->threadOpenedOn, forgottenConn);

mDBConnection is nsnull and NS_ProxyRelease is trying to use it.

See bug 556631 comment 11 and following.

Comment 1

[Marco Bonardo [:mak]]

So just replacing mDBConnection->threadOpenedOn with forgottenConn solves the problem. but I'm unsure if there are other changes wanted based on Asuth's comments. Also I have no idea how to write a test to touch this code.

Comment 2

[Marco Bonardo [:mak]]

I meant replacing with forgottenConn->threadOpenedOn

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

That certainly sounds reasonable to me, especially given that that is what the code was meant to do.

One could write a test by having a C++ test where we spin up 2 non-main threads... one opens the connection, and then the other one issues an async statement.  If we don't assert/crash, then all is well.

I'm not sure if sdwilsh wants that level of coverage or to claim that we support such a use case flawlessly though. :)

Comment 4

[Shawn Wilsher :sdwilsh]

(In reply to comment #3)
> I'm not sure if sdwilsh wants that level of coverage or to claim that we
> support such a use case flawlessly though. :)
I don't think I care about it that much.

Comment 5

[Marco Bonardo [:mak]]

Attachment: patch v1.0

I seem to understand testing this edge case is unneeded.

Comment 6

[Marco Bonardo [:mak]]

Attachment: patch v1.0

gah, the right patch.

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 476062 [details] [diff] [review]
patch v1.0

Thanks for the fix!

Comment 8

[Marco Bonardo [:mak]]

Comment on attachment 476062 [details] [diff] [review]
patch v1.0

Requesting approval on this one-line change, it is blocking my work on a blocker.

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/5bb4f093a50b