RESOLVED FIXED 101618
[Chromium] http/tests/inspector/indexeddb/database-data.html ASSERT on Win7 following r133855
https://bugs.webkit.org/show_bug.cgi?id=101618
Summary [Chromium] http/tests/inspector/indexeddb/database-data.html ASSERT on Win7 f...
Joshua Bell
Reported 2012-11-08 09:40:54 PST
Flakiness dashboard: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=http%2Ftests%2Finspector%2Findexeddb%2Fdatabase-data.html Changeset: http://trac.webkit.org/changeset/133855 Assert: STDERR: ASSERTION FAILED: active != m_active (Stack is useless, unfortunately)
Attachments
Patch (9.28 KB, patch)
2013-03-11 11:32 PDT, Alec Flett
no flags
Patch (3.18 KB, patch)
2013-03-18 11:04 PDT, Alec Flett
no flags
Patch for landing (9.66 KB, patch)
2013-03-18 12:59 PDT, Alec Flett
no flags
Patch for landing (2.67 KB, patch)
2013-03-18 13:23 PDT, Alec Flett
no flags
Vsevolod Vlasov
Comment 1 2012-11-12 07:34:00 PST
This is apparently ASSERT(active != m_active); in void IDBTransaction::setActive(bool active) Do you have an idea how can this happen?
Joshua Bell
Comment 2 2012-11-12 09:04:48 PST
(In reply to comment #1) > This is apparently ASSERT(active != m_active); in > void IDBTransaction::setActive(bool active) > > Do you have an idea how can this happen? Transactions are "active" when created (so requests can be filed against them), but go "inactive" when control returns to the event loop. When an event is later dispatched in IDBRequest::dispatchEvent the transaction is temporarily made active, the event is dispatched, then the transaction is made inactive again. So the two likely causes are: (1) recursion through IDBRequest::dispatchEvent (i.e. it's already active, and being made active again) (2) the transaction is not being made inactive when control returns to the event loop My guess would be #2, since it is currently done by having the script engine call into IDBPendingTransactionMonitor::deactivateNewTransactions() - e.g. in bindings/v8/V8RecursionScope.cpp In the inspector, this could be done manually e.g. at the end of DataLoader::execute it could call idbTransaction->setActive(false). I'd do this by introducing a stack-allocated TransactionScope class with ~TransactionScope() { m_transaction->setActive(false); } and initialize the scope after the transaction is returned from transactionForDatabase().
Vsevolod Vlasov
Comment 3 2012-11-21 06:28:43 PST
This test is not failing any mroe and according to blamelist from the dashboard http://trac.webkit.org/log/?verbose=on&rev=134988&stop_rev=134516 this was likely fixed in https://bugs.webkit.org/show_bug.cgi?id=102450. Josh, do you think the fix you mentioned above is still needed?
Joshua Bell
Comment 4 2012-11-21 10:04:45 PST
(In reply to comment #3) > This test is not failing any mroe and according to blamelist from the dashboard http://trac.webkit.org/log/?verbose=on&rev=134988&stop_rev=134516 this was likely fixed in https://bugs.webkit.org/show_bug.cgi?id=102450. Weird - I'm not sure how that would affect it, given that was a tiny tweak to a recent refactor. > Josh, do you think the fix you mentioned above is still needed? In theory, yes. But if it's not failing I wouldn't touch it for now.
Robert Kroeger
Comment 5 2012-11-23 11:13:11 PST
Fails on Win7 (release) and WinXP as well.
Joshua Bell
Comment 6 2012-11-27 09:31:09 PST
(In reply to comment #5) > Fails on Win7 (release) and WinXP as well. Yeah, looks like it's happening on Linux intermittently as well. I still think my diagnosis is correct, maybe we want to land a simpler temp fix to ensure it resolves the issue i.e. call setActive() directly at the end of DataLoader::execute
David Grogan
Comment 7 2013-02-28 10:13:32 PST
This is reproducible manually as described in http://code.google.com/p/chromium/issues/detail?id=178882 Using the inspector to look at an IDB object store causes an assert to fail. I can reproduce by Resources->IndexedDB->Expand a database->Click an objectstore -> Aw snap The assertion is ASSERTION FAILED: active != (m_state == Active) at IDBTransaction.cpp(205). I added some debug logging at the failure point: m_state = 1, Active=1. So an active transaction is being set active. Alec independently concluded that the solution was to call setActive() at the end of DataLoader::execute.
Alec Flett
Comment 8 2013-03-11 11:30:00 PDT
*** Bug 111784 has been marked as a duplicate of this bug. ***
Alec Flett
Comment 9 2013-03-11 11:32:50 PDT
Vsevolod Vlasov
Comment 10 2013-03-12 13:29:25 PDT
I don't get it: this bug is about layout test http/tests/inspector/indexeddb/database-data.html failing on win debug. We assume this is caused by incorrect "active" state of the transaction triggered by inspector. Your patch attempts to fix it, hence it should fix the test, meaning this behavior would be covered by this test. The change in resources-panel.html test that you add here is not adding any new test coverage except for the inspector front-end as far as I can see, database-data.html test is already loading data form object store.
Vsevolod Vlasov
Comment 11 2013-03-13 23:59:27 PDT
Comment on attachment 192519 [details] Patch r- per my comment above. I think InspectorIndexedDBAgent.cpp is all we really need here.
Vsevolod Vlasov
Comment 12 2013-03-13 23:59:54 PDT
(In reply to comment #11) > (From update of attachment 192519 [details]) > r- per my comment above. I think InspectorIndexedDBAgent.cpp is all we really need here. Sprry, "I think InspectorIndexedDBAgent.cpp CHANGE is all we really need here."
Alec Flett
Comment 13 2013-03-18 11:04:32 PDT
Alec Flett
Comment 14 2013-03-18 11:05:14 PDT
vsevik@ - I'm sorry you're right. I didn't realize there was a Crash expectation set for this test.
Vsevolod Vlasov
Comment 15 2013-03-18 12:07:18 PDT
Comment on attachment 193609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193609&action=review Please update ChangeLog before landing. > Source/WebCore/ChangeLog:16 > + * inspector/front-end/IndexedDBModel.js: Add new event type. There is no any changes in inspector front-end anymore.
Alec Flett
Comment 16 2013-03-18 12:59:01 PDT
Created attachment 193635 [details] Patch for landing
Alec Flett
Comment 17 2013-03-18 13:23:33 PDT
Created attachment 193637 [details] Patch for landing
WebKit Review Bot
Comment 18 2013-03-18 13:51:05 PDT
Comment on attachment 193637 [details] Patch for landing Clearing flags on attachment: 193637 Committed r146116: <http://trac.webkit.org/changeset/146116>
WebKit Review Bot
Comment 19 2013-03-18 13:51:10 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.