Changeset 248800 in webkit
- Timestamp:
- Aug 16, 2019 3:49:26 PM (5 years ago)
- Location:
- trunk
- Files:
-
- 1 added
- 3 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/JSTests/ChangeLog
r248798 r248800 1 2019-08-16 Mark Lam <mark.lam@apple.com> 2 3 CodeBlock destructor should clear all of its watchpoints. 4 https://bugs.webkit.org/show_bug.cgi?id=200792 5 <rdar://problem/53947800> 6 7 Reviewed by Yusuke Suzuki. 8 9 * stress/codeblock-should-clear-watchpoints-on-destruction.js: Added. 10 1 11 2019-08-16 Justin Michaud <justin_michaud@apple.com> 2 12 -
trunk/Source/JavaScriptCore/ChangeLog
r248798 r248800 1 2019-08-16 Mark Lam <mark.lam@apple.com> 2 3 CodeBlock destructor should clear all of its watchpoints. 4 https://bugs.webkit.org/show_bug.cgi?id=200792 5 <rdar://problem/53947800> 6 7 Reviewed by Yusuke Suzuki. 8 9 We need to clear the watchpoints explicitly (just like we do in CodeBlock::jettison()) 10 because the JITCode may outlive the CodeBlock for a while. For example, the JITCode 11 is ref'd in Interpreter::execute(JSC::CallFrameClosure&) like so: 12 13 JSValue result = closure.functionExecutable->generatedJITCodeForCall()->execute(&vm, closure.protoCallFrame); 14 15 The call to generatedJITCodeForCall() returns a Ref<JITCode> with the underlying 16 JITCode ref'd. Hence, while the interpreter frame is still on the stack, the 17 executing JITCode instance will have a non-zero refCount, and be kept alive even 18 though its CodeBlock may have already been destructed. 19 20 Note: the Interpreter execute() methods aren't the only ones who would ref the JITCode: 21 ExecutableBase also holds a RefPtr<JITCode> m_jitCodeForCall and RefPtr<JITCode> 22 m_jitCodeForConstruct. But a CodeBlock will be uninstalled before it gets destructed. 23 Hence, the uninstallation will deref the JITCode before we get to the CodeBlock 24 destructor. That said, we should be aware that a JITCode's refCount is not always 25 1 after the JIT installs it into the CodeBlock, and it should not be assumed to be so. 26 27 For this patch, I also audited all Watchpoint subclasses to ensure that we are 28 clearing all the relevant watchpoints in the CodeBlock destructor. Here is the 29 list of audited Watchpoints: 30 31 CodeBlockJettisoningWatchpoint 32 AdaptiveStructureWatchpoint 33 AdaptiveInferredPropertyValueWatchpoint 34 - these are held in the DFG::CommonData, and is tied to JITCode's life cycle. 35 - they need to be cleared eagerly in CodeBlock's destructor. 36 37 LLIntPrototypeLoadAdaptiveStructureWatchpoint 38 - stored in m_llintGetByIdWatchpointMap in the CodeBlock. 39 - this will be automatically cleared on CodeBlock destruction. 40 41 The following does not reference CodeBlock: 42 43 FunctionRareData::AllocationProfileClearingWatchpoint 44 - stored in FunctionRareData and will be cleared automatically on 45 FunctionRareData destruction. 46 - only references the owner FunctionRareData. 47 48 ObjectToStringAdaptiveStructureWatchpoint 49 ObjectToStringAdaptiveInferredPropertyValueWatchpoint 50 - stored in StructureRareData and will be cleared automatically on 51 StructureRareData destruction. 52 53 ObjectPropertyChangeAdaptiveWatchpoint 54 - stored in JSGlobalObject, and will be cleared automatically on 55 JSGlobalObject destruction. 56 - only references the owner JSGlobalObject. 57 58 StructureStubClearingWatchpoint 59 - stored in WatchpointsOnStructureStubInfo and will be cleared automatically 60 on WatchpointsOnStructureStubInfo destruction. 61 62 PropertyWatchpoint 63 StructureWatchpoint 64 - embedded in AdaptiveInferredPropertyValueWatchpointBase, which is extended 65 as AdaptiveInferredPropertyValueWatchpoint, ObjectPropertyChangeAdaptiveWatchpoint, 66 and ObjectToStringAdaptiveInferredPropertyValueWatchpoint. 67 - life cycle is handled by those 3 subclasses. 68 69 * bytecode/CodeBlock.cpp: 70 (JSC::CodeBlock::~CodeBlock): 71 1 72 2019-08-16 Justin Michaud <justin_michaud@apple.com> 2 73 -
trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp
r248027 r248800 812 812 VM& vm = *m_vm; 813 813 814 #if ENABLE(DFG_JIT) 815 // The JITCode (and its corresponding DFG::CommonData) may outlive the CodeBlock by 816 // a short amount of time after the CodeBlock is destructed. For example, the 817 // Interpreter::execute methods will ref JITCode before invoking it. This can 818 // result in the JITCode having a non-zero refCount when its owner CodeBlock is 819 // destructed. 820 // 821 // Hence, we cannot rely on DFG::CommonData destruction to clear these now invalid 822 // watchpoints in a timely manner. We'll ensure they are cleared here eagerly. 823 // 824 // We only need to do this for a DFG/FTL CodeBlock because only these will have a 825 // DFG:CommonData. Hence, the LLInt and Baseline will not have any of these watchpoints. 826 // 827 // Note also that the LLIntPrototypeLoadAdaptiveStructureWatchpoint is also related 828 // to the CodeBlock. However, its lifecycle is tied directly to the CodeBlock, and 829 // will be automatically cleared when the CodeBlock destructs. 830 831 if (JITCode::isOptimizingJIT(jitType())) 832 jitCode()->dfgCommon()->clearWatchpoints(); 833 #endif 814 834 vm.heap.codeBlockSet().remove(this); 815 835
Note: See TracChangeset
for help on using the changeset viewer.