Skip to content

Commit

Permalink
Avoid deadlock caused by Cocoa code taking the ROOT lock
Browse files Browse the repository at this point in the history 
In the example below, thread #11 is waiting on the ‘AppKit lock’ while thread #1 is waiting on the ROOT read lock and thread #12 is waiting on the ROOT write lock.

In thread #11, the ROOT write lock is taken/held by frame #9, TCanvas::Update, to ‘serialize’ the update to the windowing system.

In thread #1, the ‘AppKit lock’ is likely taken/held by a frame in the #16 to root-project#40 range.

This commit remove the dead lock by removing the unnecessary use of code needing the ROOT lock within code run under the AppKit lock so that in they case thread #1 no longer need to wait for the ROOT (read) lock.

In addition it should be investigated whether the ROOT (write) lock should be taken in thread #1, frame 1 (TMacOSXSystem::ProcessPendingEvents) for the same reason it is taken in TCanvas::Update.

  thread #11
    frame #0: 0x00007fffab109bf2 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fffab1f57fa libsystem_pthread.dylib`_pthread_cond_wait + 712
    frame #2: 0x00007fff93394e34 AppKit`-[NSViewHierarchyLock lockForReadingWithExceptionHandler:] + 287
    frame #3: 0x00007fff934948ae AppKit`-[NSWindow _copyAcquiredViewHierarchyLock] + 126
    frame #4: 0x00007fff9349442c AppKit`-[NSView lockFocusIfCanDraw] + 159
    frame #5: 0x000000011c09063d libGCocoa.so`ROOT::MacOSX::X11::CommandBuffer::Flush(this=0x0000000100dbb080, impl=0x0000000100dbb000) at X11Buffer.mm:550
    frame #6: 0x000000011c04e9c4 libGCocoa.so`TGCocoa::Update(this=0x0000000100ad1bc0, mode=1) at TGCocoa.mm:536
    frame #7: 0x000000011c04ff3e libGCocoa.so`TGCocoa::UpdateWindow(this=0x0000000100ad1bc0, (null)=1) at TGCocoa.mm:776
    frame #8: 0x000000011ad70827 libGpad.so`TCanvas::Flush(this=0x000000012274e740) at TCanvas.cxx:1096
    frame #9: 0x000000011ad7830f libGpad.so`TCanvas::Update(this=0x000000012274e740) at TCanvas.cxx:2287
    frame #10: 0x0000000100fe4e86 threadsh2_C.so`handle2((null)=0x0000000000000001) at threadsh2.C:105
    frame #11: 0x0000000100f55680 libThread.so`TThread::Function(ptr=0x0000000122753b00) at TThread.cxx:821
    frame #12: 0x00007fffab1f493b libsystem_pthread.dylib`_pthread_body + 180
    frame #13: 0x00007fffab1f4887 libsystem_pthread.dylib`_pthread_start + 286
    frame #14: 0x00007fffab1f408d libsystem_pthread.dylib`thread_start + 13

  thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
    frame #0: 0x00007fffab109bf2 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fffab1f57fa libsystem_pthread.dylib`_pthread_cond_wait + 712
    frame #2: 0x00007fffa9b734cd libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 47
    frame #3: 0x0000000100f76b6f libThread.so`void std::__1::condition_variable_any::wait<std::__1::unique_lock<std::__1::mutex> >(this=0x0000000122753a28, __lock=0x00007fff5fbf5cf8) at condition_variable:202
    frame #4: 0x0000000100f6887e libThread.so`ROOT::TReentrantRWLock<std::__1::mutex, ROOT::Internal::RecurseCounts>::ReadLock() [inlined] void std::__1::condition_variable_any::wait<std::__1::unique_lock<std::__1::mutex>, ROOT::TReentrantRWLock<std::__1::mutex, ROOT::Internal::RecurseCounts>::ReadLock()::'lambda'()>(this=0x0000000122753a28, __lock=0x00007fff5fbf5cf8, __pred=(anonymous class) @ 0x00007fff5fbf5e08)::'lambda'()) at condition_variable:211
    frame #5: 0x0000000100f68857 libThread.so`ROOT::TReentrantRWLock<std::__1::mutex, ROOT::Internal::RecurseCounts>::ReadLock(this=0x00000001227539d8) at TReentrantRWLock.cxx:95
    frame #6: 0x0000000100f5c719 libThread.so`ROOT::TRWMutexImp<std::__1::mutex, ROOT::Internal::RecurseCounts>::ReadLock(this=0x00000001227539d0) at TRWMutexImp.cxx:33
    frame #7: 0x00000001000f52d1 libCore.so`ROOT::TReadLockGuard::TReadLockGuard(this=0x00007fff5fbf5f48, mutex=0x00000001227539d0) at TVirtualRWMutex.h:89
    frame #8: 0x00000001000f275d libCore.so`ROOT::TReadLockGuard::TReadLockGuard(this=0x00007fff5fbf5f48, mutex=0x00000001227539d0) at TVirtualRWMutex.h:88
    frame #9: 0x00000001002fcebf libCore.so`THashTable::FindObject(this=0x0000000100b0e120, name="TGTextView") const at THashTable.cxx:242
    frame #10: 0x00000001003a67fd libCore.so`TClass::GetClass(name="TGTextView", load=true, silent=true) at TClass.cxx:2900
    frame #11: 0x00000001003c9e16 libCore.so`TClass::InheritsFrom(this=0x0000000118fe1250, classname="TGTextView") const at TClass.cxx:4683
    frame #12: 0x000000010024cb6e libCore.so`TObject::InheritsFrom(this=0x0000000126908de0, classname="TGTextView") const at TObject.cxx:445
    frame #13: 0x000000011c03e47f libGCocoa.so`ROOT::MacOSX::X11::ViewIsTextView(viewID=116) at QuartzWindow.mm:899
    frame #14: 0x000000011c03e518 libGCocoa.so`ROOT::MacOSX::X11::ViewIsTextView(view=0x0000000126908ee0) at QuartzWindow.mm:907
    frame #15: 0x000000011c04706c libGCocoa.so`::-[QuartzView drawRect:](self=0x0000000126908ee0, _cmd="drawRect:", dirtyRect=(origin = (x = 0, y = 0), size = (width = 29, height = 21))) at QuartzWindow.mm:2728
    frame #16: 0x00007fff934a4f99 AppKit`-[NSView _drawRect:clip:] + 2276
    frame #17: 0x00007fff934f4f2f AppKit`-[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 1753
    frame #18: 0x00007fff934f539a AppKit`-[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 2884
    frame #19: 0x00007fff934f539a AppKit`-[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 2884
    frame #20: 0x00007fff934f539a AppKit`-[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 2884
    frame #21: 0x00007fff934a2ad2 AppKit`-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 837
    frame #22: 0x00007fff934a22af AppKit`-[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 334
    frame root-project#23: 0x00007fff934a06d8 AppKit`-[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] + 2452
    frame root-project#24: 0x00007fff9349bfca AppKit`-[NSView displayIfNeeded] + 1748
    frame root-project#25: 0x00007fff9349b8db AppKit`-[NSWindow displayIfNeeded] + 230
    frame root-project#26: 0x00007fff93bfbcb4 AppKit`___NSWindowGetDisplayCycleObserver_block_invoke.6228 + 277
    frame root-project#27: 0x00007fff9349b3b9 AppKit`__37+[NSDisplayCycle currentDisplayCycle]_block_invoke + 454
    frame root-project#28: 0x00007fff9b384cc6 QuartzCore`CA::Transaction::run_commit_handlers(CATransactionPhase) + 46
    frame root-project#29: 0x00007fff9b48e8ac QuartzCore`CA::Context::commit_transaction(CA::Transaction*) + 160
    frame root-project#30: 0x00007fff9b3837a1 QuartzCore`CA::Transaction::commit() + 475
    frame root-project#31: 0x00007fff9377e8b1 AppKit`__37+[NSDisplayCycle currentDisplayCycle]_block_invoke.31 + 323
    frame root-project#32: 0x00007fff95874d37 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
    frame root-project#33: 0x00007fff95874ca7 CoreFoundation`__CFRunLoopDoObservers + 391
    frame root-project#34: 0x00007fff958556d9 CoreFoundation`__CFRunLoopRun + 873
    frame root-project#35: 0x00007fff95855114 CoreFoundation`CFRunLoopRunSpecific + 420
    frame root-project#36: 0x00007fff94db5ebc HIToolbox`RunCurrentEventLoopInMode + 240
    frame root-project#37: 0x00007fff94db5bf9 HIToolbox`ReceiveNextEventCommon + 184
    frame root-project#38: 0x00007fff94db5b26 HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 71
    frame root-project#39: 0x00007fff9334ca54 AppKit`_DPSNextEvent + 1120
    frame root-project#40: 0x00007fff93ac87ee AppKit`-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 2796
    frame root-project#41: 0x0000000100491031 libCore.so`TMacOSXSystem::ProcessPendingEvents(this=0x0000000100c06d60) at TMacOSXSystem.mm:473
    frame root-project#42: 0x000000010049097d libCore.so`TMacOSXSystem::DispatchOneEvent(this=0x0000000100c06d60, pendingOnly=true) at TMacOSXSystem.mm:365
    frame root-project#43: 0x0000000100294f4b libCore.so`TSystem::ProcessEvents(this=0x0000000100c06d60) at TSystem.cxx:429
    frame root-project#44: 0x0000000100fe5844 threadsh2_C.so`threadsh2() at threadsh2.C:196
    frame root-project#45: 0x0000000100fec06a
    frame root-project#46: 0x0000000103d7dc2f libCling.so`cling::IncrementalExecutor::executeWrapper(this=0x0000000100a1d410, function=(Data = "_Z15__cling_Un1Qu30Pv", Length = 21), returnValue=0x00007fff5fbfbde0) at IncrementalExecutor.h:196
    frame root-project#47: 0x0000000103d7db1f libCling.so`cling::Interpreter::RunFunction(this=0x0000000100a0e3b0, FD=0x000000011780f6b0, res=0x00007fff5fbfbde0) at Interpreter.cpp:980
    frame root-project#48: 0x0000000103d7a92a libCling.so`cling::Interpreter::EvaluateInternal(this=0x0000000100a0e3b0, input="threadsh2()", CO=CompilationOptions @ 0x00007fff5fbfaae8, V=0x00007fff5fbfbde0, T=0x0000000000000000, wrapPoint=44) at Interpreter.cpp:1232
    frame root-project#49: 0x0000000103d79e27 libCling.so`cling::Interpreter::process(this=0x0000000100a0e3b0, input="threadsh2()", V=0x00007fff5fbfbde0, T=0x0000000000000000, disableValuePrinting=false) at Interpreter.cpp:684
    frame root-project#50: 0x0000000103e552a5 libCling.so`cling::MetaProcessor::process(this=0x0000000100b65aa0, input_line=(Data = "threadsh2()", Length = 11), compRes=0x00007fff5fbfb540, result=0x00007fff5fbfbde0, disableValuePrinting=false) at MetaProcessor.cpp:341
    frame root-project#51: 0x000000010397bd63 libCling.so`HandleInterpreterException(metaProcessor=0x0000000100b65aa0, input_line="threadsh2()", compRes=0x00007fff5fbfb540, result=0x00007fff5fbfbde0) at TCling.cxx:2053
    frame root-project#52: 0x000000010397a16e libCling.so`TCling::ProcessLine(this=0x0000000100a0de40, line=".X  /opt/build/root_builds/master.debug/tutorials/thread/./threadsh2.C+", error=0x00007fff5fbfd694) at TCling.cxx:2170
    frame root-project#53: 0x0000000103984436 libCling.so`TCling::ProcessLineSynch(this=0x0000000100a0de40, line=".X  /opt/build/root_builds/master.debug/tutorials/thread/./threadsh2.C+", error=0x00007fff5fbfd694) at TCling.cxx:3044
    frame root-project#54: 0x00000001001f3133 libCore.so`TApplication::ExecuteFile(file="threadsh2.C+", error=0x00007fff5fbfd694, keep=false) at TApplication.cxx:1143
    frame root-project#55: 0x00000001001f19e0 libCore.so`TApplication::ProcessFile(this=0x0000000100b113e0, file="threadsh2.C+", error=0x00007fff5fbfd694, keep=false) at TApplication.cxx:1015
    frame root-project#56: 0x00000001001f138f libCore.so`TApplication::ProcessLine(this=0x0000000100b113e0, line=".x threadsh2.C+", sync=false, err=0x00007fff5fbfd694) at TApplication.cxx:988
    frame root-project#57: 0x000000010009878d libRint.so`TRint::ProcessLineNr(this=0x0000000100b113e0, filestem="ROOT_cli_", line=".x threadsh2.C+", error=0x00007fff5fbfd694) at TRint.cxx:756
    frame root-project#58: 0x0000000100097daf libRint.so`TRint::Run(this=0x0000000100b113e0, retrn=false) at TRint.cxx:416
    frame root-project#59: 0x00000001000027a4 root.exe`main(argc=1, argv=0x00007fff5fbff780) at rmain.cxx:30
    frame root-project#60: 0x00007fffaafdb235 libdyld.dylib`start + 1

  thread #12
    frame #0: 0x00007fffab109bf2 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fffab1f57fa libsystem_pthread.dylib`_pthread_cond_wait + 712
    frame #2: 0x00007fffa9b734cd libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 47
    frame #3: 0x0000000100f76b6f libThread.so`void std::__1::condition_variable_any::wait<std::__1::unique_lock<std::__1::mutex> >(this=0x0000000122753a28, __lock=0x0000700009f18898) at condition_variable:202
    frame #4: 0x0000000100f697b4 libThread.so`ROOT::TReentrantRWLock<std::__1::mutex, ROOT::Internal::RecurseCounts>::WriteLock() [inlined] void std::__1::condition_variable_any::wait<std::__1::unique_lock<std::__1::mutex>, ROOT::TReentrantRWLock<std::__1::mutex, ROOT::Internal::RecurseCounts>::WriteLock()::'lambda'()>(this=0x0000000122753a28, __lock=0x0000700009f18898, __pred=(anonymous class) @ 0x0000700009f18a28)::'lambda'()) at condition_variable:211
    frame #5: 0x0000000100f69790 libThread.so`ROOT::TReentrantRWLock<std::__1::mutex, ROOT::Internal::RecurseCounts>::WriteLock(this=0x00000001227539d8) at TReentrantRWLock.cxx:175
    frame #6: 0x0000000100f5c779 libThread.so`ROOT::TRWMutexImp<std::__1::mutex, ROOT::Internal::RecurseCounts>::WriteLock(this=0x00000001227539d0) at TRWMutexImp.cxx:42
    frame #7: 0x0000000100f57df6 libThread.so`ROOT::TVirtualRWMutex::Lock(this=0x00000001227539d0) at TVirtualRWMutex.h:52
    frame #8: 0x00000001039b2dd9 libCling.so`TLockGuard::TLockGuard(this=0x0000700009f18b48, mutex=0x00000001227539d0) at TVirtualMutex.h:85
    frame #9: 0x000000010397ba8d libCling.so`TLockGuard::TLockGuard(this=0x0000700009f18b48, mutex=0x00000001227539d0) at TVirtualMutex.h:85
    frame #10: 0x00000001039aa152 libCling.so`TCling::ClassInfo_Factory(this=0x0000000100a0de40, all=true) const at TCling.cxx:7216
    frame #11: 0x00000001004150ba libCore.so`TMethodCall::Init(this=0x0000700009f18d80, cl=0x0000000100af0470, method="Print", params="", objectIsConst=false) at TMethodCall.cxx:259
    frame #12: 0x0000000100414ff3 libCore.so`TMethodCall::TMethodCall(this=0x0000700009f18d80, cl=0x0000000100af0470, method="Print", params="") at TMethodCall.cxx:62
    frame #13: 0x000000010041519d libCore.so`TMethodCall::TMethodCall(this=0x0000700009f18d80, cl=0x0000000100af0470, method="Print", params="") at TMethodCall.cxx:61
    frame #14: 0x0000000100fe471a threadsh2_C.so`handle1((null)=0x0000000000000000) at threadsh2.C:48
    frame #15: 0x0000000100f55680 libThread.so`TThread::Function(ptr=0x0000000122754790) at TThread.cxx:821
    frame #16: 0x00007fffab1f493b libsystem_pthread.dylib`_pthread_body + 180
    frame #17: 0x00007fffab1f4887 libsystem_pthread.dylib`_pthread_start + 286
    frame #18: 0x00007fffab1f408d libsystem_pthread.dylib`thread_start + 13
  • Loading branch information
@pcanal
pcanal committed Feb 5, 2018
1 parent 8ac53b7 commit c2e3a62
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 5 deletions.
28 changes: 23 additions & 5 deletions graf2d/cocoa/src/QuartzWindow.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
#include "TSystem.h"
#include "TGCocoa.h"
#include "TROOT.h"
#include "TGTextView.h"
#include "TGView.h"
#include "TGCanvas.h"


namespace ROOT {
Expand Down Expand Up @@ -896,7 +899,10 @@ bool ViewIsTextView(unsigned viewID)
const TGWindow * const window = gClient->GetWindowById(viewID);
if (!window)
return false;
return window->InheritsFrom("TGTextView");
// This code used to use TObject::InheritsFrom, however since this is
// run under the AppKit, we can not call core/meta functions, otherwise
// we will run into deadlocks.
return dynamic_cast<const TGTextView*>(window);
}

//______________________________________________________________________________
Expand All @@ -916,7 +922,10 @@ bool ViewIsTextViewFrame(NSView<X11Window> *view, bool checkParent)
if (!window)
return false;

if (!window->InheritsFrom("TGViewFrame"))
// This code used to use TObject::InheritsFrom, however since this is
// run under the AppKit, we can not call core/meta functions, otherwise
// we will run into deadlocks.
if (!dynamic_cast<const TGViewFrame*>(window))
return false;

if (!checkParent)
Expand All @@ -934,7 +943,10 @@ bool ViewIsHtmlView(unsigned viewID)
const TGWindow * const window = gClient->GetWindowById(viewID);
if (!window)
return false;
return window->InheritsFrom("TGHtml");
// This code used to use TObject::InheritsFrom, however since this is
// run under the AppKit, we can not call core/meta functions, otherwise
// we will run into deadlocks.
return window->TestBit(TGWindow::kIsHtmlView);
}

//______________________________________________________________________________
Expand All @@ -955,7 +967,10 @@ bool ViewIsHtmlViewFrame(NSView<X11Window> *view, bool checkParent)
if (!window)
return false;

if (!window->InheritsFrom("TGViewFrame"))
// This code used to use TObject::InheritsFrom, however since this is
// run under the AppKit, we can not call core/meta functions, otherwise
// we will run into deadlocks.
if (!dynamic_cast<const TGViewFrame*>(window))
return false;

if (!checkParent)
Expand Down Expand Up @@ -2705,7 +2720,10 @@ - (void) drawRect : (NSRect) dirtyRect
if (self.fQuartzWindow.fShapeCombineMask)
X11::ClipToShapeMask(self, fContext);

if (window->InheritsFrom("TGContainer"))//It always has an ExposureMask.
// This code used to use TObject::InheritsFrom, however since this is
// run under the AppKit, we can not call core/meta functions, otherwise
// we will run into deadlocks.
if (dynamic_cast<const TGContainer*>(window))//It always has an ExposureMask.
vx->GetEventTranslator()->GenerateExposeEvent(self, [self visibleRect]);

if (fEventMask & kExposureMask) {
Expand Down
4 changes: 4 additions & 0 deletions gui/gui/inc/TGWindow.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ friend class TGClient;
kEditDisableKeyEnable = BIT(8) // window can handle keyboard events
};

enum EStatusBits {
kIsHtmlView = BIT(14)
};

TGWindow(const TGWindow *p = 0, Int_t x = 0, Int_t y = 0,
UInt_t w = 0, UInt_t h = 0, UInt_t border = 0,
Int_t depth = 0,
Expand Down
2 changes: 2 additions & 0 deletions gui/guihtml/src/TGHtml.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ int HtmlDepth = 0;

TGHtml::TGHtml(const TGWindow *p, int w, int h, int id) : TGView(p, w, h, id)
{
SetBit(kIsHtmlView);

int i;

fExiting = 0;
Expand Down

0 comments on commit c2e3a62

Please sign in to comment.