- Revision
- 219114
- Author
- [email protected]
- Date
- 2017-07-03 22:18:15 -0700 (Mon, 03 Jul 2017)
Log Message
LayoutTest workers/bomb.html is a Crash
https://bugs.webkit.org/show_bug.cgi?id=167757
<rdar://problem/33086462>
Reviewed by Keith Miller.
Source/_javascript_Core:
VMTraps::SignalSender was accessing VM fields even after
the VM was destroyed. This happened when the SignalSender
thread was in the middle of its work() function while VMTraps
was notified that the VM was shutting down. The VM would proceed
to run its destructor even after the SignalSender thread finished
doing its work. This means that the SignalSender thread was accessing
VM field eve after VM was destructed (including itself, since it is
transitively owned by the VM). The VM must wait for the SignalSender
thread to shutdown before it can continue to destruct itself.
* runtime/VMTraps.cpp:
(JSC::VMTraps::willDestroyVM):
Source/WTF:
* wtf/AutomaticThread.cpp:
(WTF::AutomaticThreadCondition::waitFor):
* wtf/AutomaticThread.h:
LayoutTests:
* platform/mac-wk2/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (219113 => 219114)
--- trunk/LayoutTests/ChangeLog 2017-07-04 04:33:21 UTC (rev 219113)
+++ trunk/LayoutTests/ChangeLog 2017-07-04 05:18:15 UTC (rev 219114)
@@ -1,3 +1,13 @@
+2017-07-03 Saam Barati <[email protected]>
+
+ LayoutTest workers/bomb.html is a Crash
+ https://bugs.webkit.org/show_bug.cgi?id=167757
+ <rdar://problem/33086462>
+
+ Reviewed by Keith Miller.
+
+ * platform/mac-wk2/TestExpectations:
+
2017-07-03 Matt Lewis <[email protected]>
Removed expectations and skipped workers/bomb.html on mac.
Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (219113 => 219114)
--- trunk/LayoutTests/platform/mac-wk2/TestExpectations 2017-07-04 04:33:21 UTC (rev 219113)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations 2017-07-04 05:18:15 UTC (rev 219114)
@@ -706,5 +706,5 @@
webkit.org/b/173608 webrtc/video-replace-muted-track.html [ Pass Failure ]
-webkit.org/b/167757 workers/bomb.html [ Skip ]
+webkit.org/b/167757 workers/bomb.html [ Pass Timeout ]
Modified: trunk/Source/_javascript_Core/ChangeLog (219113 => 219114)
--- trunk/Source/_javascript_Core/ChangeLog 2017-07-04 04:33:21 UTC (rev 219113)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-07-04 05:18:15 UTC (rev 219114)
@@ -1,5 +1,26 @@
2017-07-03 Saam Barati <[email protected]>
+ LayoutTest workers/bomb.html is a Crash
+ https://bugs.webkit.org/show_bug.cgi?id=167757
+ <rdar://problem/33086462>
+
+ Reviewed by Keith Miller.
+
+ VMTraps::SignalSender was accessing VM fields even after
+ the VM was destroyed. This happened when the SignalSender
+ thread was in the middle of its work() function while VMTraps
+ was notified that the VM was shutting down. The VM would proceed
+ to run its destructor even after the SignalSender thread finished
+ doing its work. This means that the SignalSender thread was accessing
+ VM field eve after VM was destructed (including itself, since it is
+ transitively owned by the VM). The VM must wait for the SignalSender
+ thread to shutdown before it can continue to destruct itself.
+
+ * runtime/VMTraps.cpp:
+ (JSC::VMTraps::willDestroyVM):
+
+2017-07-03 Saam Barati <[email protected]>
+
DFGBytecodeParser op_to_this does not access the correct instruction offset for to this status
https://bugs.webkit.org/show_bug.cgi?id=174110
Modified: trunk/Source/_javascript_Core/runtime/VMTraps.cpp (219113 => 219114)
--- trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2017-07-04 04:33:21 UTC (rev 219113)
+++ trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2017-07-04 05:18:15 UTC (rev 219114)
@@ -254,8 +254,6 @@
WorkResult work() override
{
-
- // We need a nested scope so that we'll release the lock before we sleep below.
VM& vm = m_vm;
auto optionalOwnerThread = vm.ownerThread();
@@ -291,7 +289,12 @@
});
}
- sleepMS(1);
+ {
+ auto locker = holdLock(*traps().m_lock);
+ if (traps().m_isShuttingDown)
+ return WorkResult::Stop;
+ traps().m_trapSet->waitFor(*traps().m_lock, 1_ms);
+ }
return WorkResult::Continue;
}
@@ -305,7 +308,6 @@
void VMTraps::willDestroyVM()
{
m_isShuttingDown = true;
- WTF::storeStoreFence();
#if ENABLE(SIGNAL_BASED_VM_TRAPS)
if (m_signalSender) {
{
@@ -313,8 +315,7 @@
if (!m_signalSender->tryStop(locker))
m_trapSet->notifyAll(locker);
}
- if (!ASSERT_DISABLED)
- m_signalSender->join();
+ m_signalSender->join();
m_signalSender = nullptr;
}
#endif
Modified: trunk/Source/WTF/ChangeLog (219113 => 219114)
--- trunk/Source/WTF/ChangeLog 2017-07-04 04:33:21 UTC (rev 219113)
+++ trunk/Source/WTF/ChangeLog 2017-07-04 05:18:15 UTC (rev 219114)
@@ -1,3 +1,15 @@
+2017-07-03 Saam Barati <[email protected]>
+
+ LayoutTest workers/bomb.html is a Crash
+ https://bugs.webkit.org/show_bug.cgi?id=167757
+ <rdar://problem/33086462>
+
+ Reviewed by Keith Miller.
+
+ * wtf/AutomaticThread.cpp:
+ (WTF::AutomaticThreadCondition::waitFor):
+ * wtf/AutomaticThread.h:
+
2017-07-03 Commit Queue <[email protected]>
Unreviewed, rolling out r219060.
Modified: trunk/Source/WTF/wtf/AutomaticThread.cpp (219113 => 219114)
--- trunk/Source/WTF/wtf/AutomaticThread.cpp 2017-07-04 04:33:21 UTC (rev 219113)
+++ trunk/Source/WTF/wtf/AutomaticThread.cpp 2017-07-04 05:18:15 UTC (rev 219114)
@@ -81,6 +81,11 @@
m_condition.wait(lock);
}
+bool AutomaticThreadCondition::waitFor(Lock& lock, Seconds time)
+{
+ return m_condition.waitFor(lock, time);
+}
+
void AutomaticThreadCondition::add(const AbstractLocker&, AutomaticThread* thread)
{
ASSERT(!m_threads.contains(thread));
Modified: trunk/Source/WTF/wtf/AutomaticThread.h (219113 => 219114)
--- trunk/Source/WTF/wtf/AutomaticThread.h 2017-07-04 04:33:21 UTC (rev 219113)
+++ trunk/Source/WTF/wtf/AutomaticThread.h 2017-07-04 05:18:15 UTC (rev 219114)
@@ -82,6 +82,7 @@
// threads. In such cases, the thread doing the notifyAll() can wake up at most one thread -
// its partner.
WTF_EXPORT_PRIVATE void wait(Lock&);
+ WTF_EXPORT_PRIVATE bool waitFor(Lock&, Seconds);
private:
friend class AutomaticThread;
