Closed
Bug 711076
Opened 11 years ago
Closed 11 years ago
Check that we call onnection::internalClose on every connection we called Connection::initialize on
Categories
(Toolkit :: Storage, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: espindola, Assigned: espindola)
References
Details
Attachments
(1 file, 2 obsolete files)
1.14 KB,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•11 years ago
|
||
Now that I think of it, sOpenConnectionCount should probably be a file static. The MOZ_ASSERT might also have to be an NS_ASSERTION for now. I will add a reviewer once I have the try results. https://tbpl.mozilla.org/?tree=Try&rev=1c9e56225e3d
Assignee | ||
Comment 2•11 years ago
|
||
An updated patch is at https://tbpl.mozilla.org/?tree=Try&rev=a6fd4b6509df It include the fixes for bugs 711447 and 711494.
Comment 3•11 years ago
|
||
I don't understand why we need this. Can you elaborate?
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Shawn Wilsher :sdwilsh (Vacation Dec 17 - Jan 5) from comment #3) > I don't understand why we need this. Can you elaborate? It is a sanity check. Making sure connections are closed makes sure that all relevant statements are finalized and the corresponding threads are shut down. A thread that sometimes survives too long is the cause of 673017 for example.
Assignee | ||
Comment 5•11 years ago
|
||
I pushed a new combined patch to https://tbpl.mozilla.org/?tree=Try&rev=0209f9c7d338
Assignee | ||
Comment 6•11 years ago
|
||
And now with a missing null check added: https://tbpl.mozilla.org/?tree=Try&rev=c7e4b3f4413a
Assignee | ||
Comment 7•11 years ago
|
||
The xpcshell failure was because places was not spinning. A new push is at https://tbpl.mozilla.org/?tree=Try&rev=f8b24142c8ed If this is green I will split and send for review.
Assignee | ||
Comment 8•11 years ago
|
||
A new combined patch is at https://tbpl.mozilla.org/?tree=Try&rev=398cde3291f2
Assignee | ||
Comment 9•11 years ago
|
||
New try push at https://tbpl.mozilla.org/?tree=Try&rev=d5f7d7218ecc
Updated•11 years ago
|
Whiteboard: [snappy]
Assignee | ||
Comment 10•11 years ago
|
||
New try push at https://tbpl.mozilla.org/?tree=Try&rev=d8591f907567
Assignee | ||
Comment 11•11 years ago
|
||
new push at https://tbpl.mozilla.org/?tree=Try&rev=ad06f61d0f5d
Assignee | ||
Comment 12•11 years ago
|
||
and now on top of a working revision: https://tbpl.mozilla.org/?tree=Try&rev=807c02faf7c5
Comment 13•11 years ago
|
||
No serious need to mark correctness fixes as snappy if they block existing snappy bugs.
Blocks: 662444
Whiteboard: [snappy]
Assignee | ||
Comment 14•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=29cd4aa0c1d0
Attachment #582112 -
Attachment is obsolete: true
Attachment #623190 -
Flags: review?(mak77)
Assignee | ||
Comment 15•11 years ago
|
||
ping
Comment 16•11 years ago
|
||
Comment on attachment 623190 [details] [diff] [review] check that all connections are closed. Review of attachment 623190 [details] [diff] [review]: ----------------------------------------------------------------- will r+ next version ::: storage/src/mozStorageService.cpp @@ +884,5 @@ > + nsTArray<nsRefPtr<Connection> > connections; > + getConnections(connections); > + for (PRUint32 i = 0; i < connections.Length(); i++) { > + MOZ_ASSERT(!connections[i]->ConnectionReady()); > + } all of this is debug, since you just use MOZ_ASSERT, so you can well wrap everything into an ifdef DEBUG and avoid looping at runtime.
Attachment #623190 -
Flags: review?(mak77)
Assignee | ||
Comment 17•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=b1e62cfb96b9
Attachment #623190 -
Attachment is obsolete: true
Attachment #626775 -
Flags: review?(mak77)
Comment 18•11 years ago
|
||
Comment on attachment 626775 [details] [diff] [review] check that all connections are closed. Review of attachment 626775 [details] [diff] [review]: ----------------------------------------------------------------- ::: storage/src/mozStorageService.cpp @@ +847,5 @@ > + > +#ifdef DEBUG > + nsTArray<nsRefPtr<Connection> > connections; > + getConnections(connections); > + for (PRUint32 i = 0, n = connections.Length(); i < n; i++) { why the temporary n var?
Attachment #626775 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 19•11 years ago
|
||
> > + for (PRUint32 i = 0, n = connections.Length(); i < n; i++) {
>
> why the temporary n var?
Not calling Length at each iteration.
Comment 20•11 years ago
|
||
ah ok, thought there was some exotic reason :)
Assignee | ||
Comment 21•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=b842c7baf11c
Comment 22•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b842c7baf11c
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in
before you can comment on or make changes to this bug.
Description
•