On Mon, 28 Sep 2020 12:00:40 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:
>>> Thank you for the comments and diagrams; they make the code much more >>> digestible. From that diagram, I get the >>> impression that the watermark is associated with stack pointer, so it >>> should be 1:1 relation, but `class Thread` >>> contains multiple watermarks, `StackWatermarks _stack_watermarks;`. I think >>> some high level description on the relation >>> between the thread and a list of watermarks belong to that thread could be >>> beneficial. >> >> I added some further comments explaining this. >> >>> > The first time it reaches past the last frame it will report true, and >>> > the second time it will report false. >>> >>> Why so? As I see it, once a stream becomes "done", it stays in that state >>> forever. Am I missing sth? >>> >>> ``` >>> inline bool StackFrameStream::is_done() { >>> return (_is_done) ? true : (_is_done = _fr.is_first_frame(), false); >>> } >>> ``` >> >> When you step into the last frame of the iteration (first frame in the >> stack), the first time you ask is_done() it will >> report false, the next time it will report true, despite still being in the >> same frame. Therefore, the is_done() >> function is not idempotent, as I need it to be. > > I see; thank you for the explanation. Hi Erik, I have been playing with this patch for past a few days. Great work! I found that this patch seems to break an early assumption. We have a comment in JavaThread::exit() says: <pre> // We need to cache the thread name for logging purposes below as once // we have called on_thread_detach this thread must not access any oops. </pre> Then in method : <pre><code> void Threads::remove(JavaThread* p, bool is_daemon) { ... BarrierSet::barrier_set()->on_thread_detach(p); // Extra scope needed for Thread_lock, so we can check // that we do not remove thread without safepoint code notice { MonitorLocker ml(Threads_lock); .. } </code></pre> It calls barrier's on_thread_detach(), acquires Threads_lock. The lock acquisition triggers stack processing, that potential accesses oops. <pre><code> V [libjvm.so+0x10c6f5e] StackWatermark::start_processing()+0x6a V [libjvm.so+0x10c77e8] StackWatermarkSet::start_processing(JavaThread*, StackWatermarkKind)+0x82 V [libjvm.so+0xfd7757] SafepointMechanism::process(JavaThread*)+0x37 V [libjvm.so+0xfd796b] SafepointMechanism::process_if_requested_slow(JavaThread*)+0x1d V [libjvm.so+0x4b3683] SafepointMechanism::process_if_requested(JavaThread*)+0x2b V [libjvm.so+0xe87f0d] ThreadBlockInVMWithDeadlockCheck::~ThreadBlockInVMWithDeadlockCheck()+0x5f V [libjvm.so+0xe86700] Mutex::lock_contended(Thread*)+0x12c V [libjvm.so+0xe867d8] Mutex::lock(Thread*)+0x96 V [libjvm.so+0xe86823] Mutex::lock()+0x23 V [libjvm.so+0x29b4bc] MutexLocker::MutexLocker(Mutex*, Mutex::SafepointCheckFlag)+0xe2 V [libjvm.so+0x29b533] MonitorLocker::MonitorLocker(Monitor*, Mutex::SafepointCheckFlag)+0x29 V [libjvm.so+0x119f2ce] Threads::remove(JavaThread*, bool)+0x56 V [libjvm.so+0x1198a2b] JavaThread::exit(bool, JavaThread::ExitType)+0x905 V [libjvm.so+0x1197fde] JavaThread::post_run()+0x22 V [libjvm.so+0x1193eae] Thread::call_run()+0x230 V [libjvm.so+0xef3e38] thread_native_entry(Thread*)+0x1e4 </code></pre> This is a problem for Shenandoah, as it flushes SATB buffers during on_thread_detach() and does not expect to see any more SATB traffic. Thanks. ------------- PR: https://git.openjdk.java.net/jdk/pull/296