✨ อัปเดตล่าสุด: eclipse omr/omr/pull/

RSalman · 2022-08-03T17:51:24Z

With Concurrent Scavenger, we may end up with unflushed Ownabele Sync. buffers which can eventually lead to a heap corruption. Multiple crashes and assertion have been reported as a result of this, for instance:

  j9mm.141    *   ** ASSERTION FAILED ** at
./OwnableSynchronizerObjectBuffer.cpp:100: ((false))

** ASSERTION FAILED ** at ./Scavenger.cpp:1659: ((false))

Corruption in Evacuate at 0000005020E395F8: calculated object size
344152650780 larger then available 3172872, Forwarded Header at
00000050322FE8A8

*   ** ASSERTION FAILED ** at ../../../gc_glue_java/MarkingDelegate.hpp:123: ((false && ((UDATA)0x99669966 == clazz->eyecatcher)))

For Concurrent Scavenger, the Ownable buffers are flushed:

At the start of a global cycle when we acquire exclusive access. https://github.com/eclipse/omr/blob/e70cf35e07900aa3dcf6f5c68f895094faadaca1/gc/base/standard/ConcurrentGC.hpp#L379
Scavenger will flush Ownable buffers for participating threads, as
threads complete a cycle, scavengerRootScanner.flush(env); .
https://github.com/eclipse/omr/blob/e70cf35e07900aa3dcf6f5c68f895094faadaca1/gc/base/standard/Scavenger.cpp#L5634

In terms of #⁠2, the scheduler may dispatch different threads for different phases of CS; Ownable lists are built during scan phase, however buffers are only flushed during the final phase. Hence, threads that don't participate in the last phase (but participated in prior phases) may still have buffers which need to be flushed. Typically, this isn't an issue because all buffers will be flushed as we acquire exclusive for global (#⁠1).

The problem arises as there's still a possibility to miss flushing the buffers: when we complete a CS cycle as part of a global cycle (i.e CS cycle in STW through MM_ParallelGlobalGC::completeExternalConcurrentCycle). In which case, we're already past acquiring exlusive (#⁠1) and scavenger (#⁠2) will miss to flush threads that don't participate in the final phase of CS (but built lists in prior phases).

A simple workaround is to invoke GC_OMRVMInterface::flushNonAllocationCaches(env); in this specific
case. This will ensure all thread buffers are flushed. This should be revisited and a better solution should be adopted for Scavenger to handle this.

Signed-off-by: Salman Rana salman.rana@ibm.com

RSalman · 2022-08-03T17:58:47Z

@dmitripivkine @amicic please review

With Concurrent Scavenger, we may end up with unflushed Ownabele Sync. buffers which can lead to heap corruption. Multiple crashes and assertion have been reported as a result of this, for instance: ``` j9mm.141 * ** ASSERTION FAILED ** at ./OwnableSynchronizerObjectBuffer.cpp:100: ((false)) ``` ``` ** ASSERTION FAILED ** at ./Scavenger.cpp:1659: ((false)) ``` ``` Corruption in Evacuate at 0000005020E395F8: calculated object size 344152650780 larger then available 3172872, Forwarded Header at 00000050322FE8A8 ``` ``` * ** ASSERTION FAILED ** at ../../../gc_glue_java/MarkingDelegate.hpp:123: ((false && ((UDATA)0x99669966 == clazz->eyecatcher))) ``` Ownable buffers are flushed: 1. At the start of a global cycle when we acquire exlusive access. 2. Scavenger will flush Ownable buffers for participating threads, as threads complete a cycle, `scavengerRootScanner.flush(env);` . In terms of #⁠2, the scheduler may dispatch different threads for different phases of CS; Ownable lists are built during scan phase, however buffers are only flushed during the final phase. Hence, threads that don't participate in the last phase (but participated in prior phases) may still have buffers which need to be flushed. Typically, this isn't an issue because all buffers will be flushed as we acquire exclusive for global (#⁠1). The problem arises because there's still a possibility to miss flushing the buffers: when we complete a CS cycle as part of a global cycle (i.e CS cycle in STW through `MM_ParallelGlobalGC::completeExternalConcurrentCycle`). In which case, we're already past acquiring exlusive (#⁠1) and scavenger (#⁠2) will miss to flush threads that don't participate in the final phase of CS (but built lists in prior phases). A simple workaround is to invoke `GC_OMRVMInterface::flushNonAllocationCaches(env);` in this specific case. This will ensure all thread buffers are flushed. This should be revisited and a better solution should be adopted for Scavenger to handle this. Signed-off-by: Salman Rana <salman.rana@ibm.com>

RSalman · 2022-08-03T19:56:20Z

Debug trace points without this change:

18:16:22.548255261  0x00003fffa80e0d00 j9mm.769            Event       MM_ParallelGlobalGC::completeExternalConcurrentCycle Concurrent Scavenger cycle is in progress 
18:16:22.548256261  0x00003fffa80e0d00 j9mm.148            Entry      >Scavenger start 


18:16:22.550921240 *0x00003fffa80e0d00 j9mm.536            Event       MM_ParallelDispatcher::adjustThreadCount limiting threads to 5 due to heap size

						***** SCAVENGE_SCAN Phase Start *****

18:16:22.553652218  0x00003fff88002200 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  3] [Phase: SCAVENGE_SCAN] [Ownable Buffer: NOT EMPTY]
18:16:22.553654218  0x00003fff90002200 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  1] [Phase: SCAVENGE_SCAN] [Ownable Buffer: EMPTY]
18:16:22.553658218  0x00003fff84002200 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  2] [Phase: SCAVENGE_SCAN] [Ownable Buffer: NOT EMPTY]
18:16:22.553662218  0x00003fff74002200 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  6] [Phase: SCAVENGE_SCAN] [Ownable Buffer: NOT EMPTY]
18:16:22.553675218  0x00003fffa80e0d00 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  0] [Phase: SCAVENGE_SCAN] [Ownable Buffer: NOT EMPTY]

18:16:22.553702218  0x00003fffa80e0d00 j9mm.536            Event       MM_ParallelDispatcher::adjustThreadCount limiting threads to 5 due to heap size 

						***** SCAVENGE_COMPLETE Phase Start *****

18:16:22.554612211 *0x00003fff7c002200 j9mm.766            Event       MM_Scavenger::workThreadGarbageCollect FLUSHING [env:  4]
18:16:22.554612211 *0x00003fff78002200 j9mm.766            Event       MM_Scavenger::workThreadGarbageCollect FLUSHING [env:  7]
18:16:22.554613211 *0x00003fffa80e0d00 j9mm.766            Event       MM_Scavenger::workThreadGarbageCollect FLUSHING [env:  0]
18:16:22.554614211 *0x00003fff74002200 j9mm.766            Event       MM_Scavenger::workThreadGarbageCollect FLUSHING [env:  6]
18:16:22.554629210 *0x00003fff84002200 j9mm.766            Event       MM_Scavenger::workThreadGarbageCollect FLUSHING [env:  2]
18:16:22.554620211  0x00003fff74002200 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  6] [Phase: SCAVENGE_COMPLETE] [Ownable Buffer: EMPTY]
18:16:22.554623211  0x00003fff78002200 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  7] [Phase: SCAVENGE_COMPLETE] [Ownable Buffer: EMPTY]
18:16:22.554637210  0x00003fff84002200 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  2] [Phase: SCAVENGE_COMPLETE] [Ownable Buffer: EMPTY]
18:16:22.554647210  0x00003fff7c002200 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  4] [Phase: SCAVENGE_COMPLETE] [Ownable Buffer: EMPTY]
18:16:22.554708210  0x00003fffa80e0d00 j9mm.768            Event       MM_ConcurrentScavengeTask::run [env:  0] [Phase: SCAVENGE_COMPLETE] [Ownable Buffer: EMPTY]

18:16:22.554710210 *0x00003fff88002200 j9mm.767            Event       MM_GCExtensions::checkAndVerifyOwnableSynchronizerObjectList NOT FLUSHED [env:  3]

These trace points show:

Threads: 0,1,2,3, & 6 are selected to during SCAVENGE_SCAN phase.
By the end of the phase, the only thread with empty buffer is thread 1.
Threads 0,2,3 & 6 need to be flushed.
For final phase, threads: 0,2,4,6 & 7 are selected.

Since thread 3 is not selected, its buffer won't be flushed during the final phase.

RSalman · 2022-08-05T16:26:59Z

ready to be merged - could you please proceed @0xdaryl

babsingh · 2022-08-05T16:29:04Z

jenkins build all

babsingh · 2022-08-05T17:12:56Z

https://ci.eclipse.org/omr/job/PullRequest-linux_ppc-64_le_gcc/3735/console

Known failure: compunittest AbsVPValueTest segfaults #6556

babsingh · 2022-08-05T17:17:15Z

This is a temporary workaround
This should be revisited and a better solution should be adopted for Scavenger to handle this.

Is there an issue for the long-term solution (for tracking purposes)? Can it be linked to this PR?

RSalman · 2022-08-05T19:46:37Z

We will actually be removing ownable sync. lists all together in the near future, we can revert this change after that work is completed. It is being tracked here: eclipse-openj9/openj9#15193 with the new implementation currently under review

Edit: fixed PR issue link

babsingh · 2022-08-05T19:50:54Z

eclipse-openj9/openj9#1519 seems unrelated. Should it be eclipse-openj9/openj9#15193?

RSalman · 2022-08-05T19:54:41Z

This should be revisited and a better solution should be adopted for Scavenger to handle this.

In terms of Ownable Sync., we can revert this change. However, there is still a fundamental issue(?) in terms of "consistency" with different threads being selected

RSalman · 2022-08-05T19:56:01Z

I'll be discussing with @amicic and will open an issue if required.

RSalman · 2022-08-05T19:56:57Z

eclipse-openj9/openj9#1519 seems unrelated. Should it be eclipse-openj9/openj9#15193?

Yup, I messed that up. Thanks!

Relates to eclipse-omr#6633 Currently, we rely on the global collector to flush ownable sync. buffers when we complete an external concurrent scavenge cycle during a global collection. This not optimal as it requires all the threads to be iterated and flushed. Furthermore, it is Scavenger's responsibility to flush buffers for the participating GC threads in a CS phase. Signed-off-by: Salman Rana <salman.rana@ibm.com>

RSalman requested review from charliegracie, rwy7 and youngar as code owners August 3, 2022 17:51

RSalman force-pushed the ownable-cs-fix branch from f7776c6 to ced48e4 Compare August 3, 2022 17:52

github-actions Bot added the comp:gc label Aug 3, 2022

RSalman force-pushed the ownable-cs-fix branch from ced48e4 to b84551a Compare August 3, 2022 17:57

RSalman force-pushed the ownable-cs-fix branch from b84551a to 56a16e5 Compare August 3, 2022 18:03

dmitripivkine approved these changes Aug 3, 2022

View reviewed changes

RSalman changed the title ~~WIP: Heap Corruption from Own. Sync. Lists with Concurrent Scavenger Fix~~ Fix: Heap Corruption from Ownable Synchronizers Lists with Concurrent Scavenger Aug 5, 2022

babsingh approved these changes Aug 5, 2022

View reviewed changes

babsingh merged commit 9a26997 into eclipse-omr:master Aug 5, 2022

RSalman mentioned this pull request Aug 5, 2022

Redesign of Locked Synchronizer processing eclipse-openj9/openj9#15193

Closed

RSalman mentioned this pull request Aug 11, 2022

Flush Ownable Synchronizer Buffers For Each Concurrent Scavenge Phase #6651

Merged

Navigation Menu

Uh oh!

Fix: Heap Corruption from Ownable Synchronizers Lists with Concurrent Scavenger - #6633

Fix: Heap Corruption from Ownable Synchronizers Lists with Concurrent Scavenger #6633
babsingh merged 1 commit into
eclipse-omr:masterfrom
RSalman:ownable-cs-fix

RSalman commented Aug 3, 2022 •

edited

Loading

Uh oh!

RSalman commented Aug 3, 2022

Uh oh!

RSalman commented Aug 3, 2022 •

edited

Loading

Uh oh!

RSalman commented Aug 5, 2022

Uh oh!

babsingh commented Aug 5, 2022

Uh oh!

babsingh commented Aug 5, 2022

Uh oh!

babsingh commented Aug 5, 2022

Uh oh!

RSalman commented Aug 5, 2022 •

edited

Loading

Uh oh!

babsingh commented Aug 5, 2022

Uh oh!

RSalman commented Aug 5, 2022 •

edited

Loading

Uh oh!

RSalman commented Aug 5, 2022 •

edited

Loading

Uh oh!

RSalman commented Aug 5, 2022 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

RSalman commented Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RSalman commented Aug 3, 2022

Uh oh!

RSalman commented Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RSalman commented Aug 5, 2022

Uh oh!

babsingh commented Aug 5, 2022

Uh oh!

babsingh commented Aug 5, 2022

Uh oh!

babsingh commented Aug 5, 2022

Uh oh!

RSalman commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babsingh commented Aug 5, 2022

Uh oh!

RSalman commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RSalman commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RSalman commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RSalman commented Aug 3, 2022 •

edited

Loading

RSalman commented Aug 3, 2022 •

edited

Loading

RSalman commented Aug 5, 2022 •

edited

Loading

RSalman commented Aug 5, 2022 •

edited

Loading

RSalman commented Aug 5, 2022 •

edited

Loading

RSalman commented Aug 5, 2022 •

edited

Loading