[9] RFR(S): 8075214: SIGSEGV in nmethod sweeping (original) (raw)

David Holmes david.holmes at oracle.com
Fri Mar 27 08:51:38 UTC 2015

Correction ...

On 27/03/2015 3:25 PM, David Holmes wrote:

Hi Tobias,

On 26/03/2015 10:52 PM, Tobias Hartmann wrote: Hi Vladimir,

On 25.03.2015 18:10, Vladimir Kozlov wrote: This is messy. Tobias, can you investigate if we can have the same functionality (sweeping on request) in already running sweeper thread without crating new thread for that? It may require additional synchronization which should be fine since it is testing - no need to worry about performance. I agree. I removed creation of a new sweeper thread and added the method NMethodSweeper::forcesweep() to enforce a sweep by setting 'forcesweep', notifying the sweeper and waiting for completion. Synchronization is done by using the CodeCachelock. Here are the new webrevs: http://cr.openjdk.java.net/~thartmann/8075214/top/webrev.00/ http://cr.openjdk.java.net/~thartmann/8075214/webrev.01/ This won't quite work. Suppose the sweeper thread is active in possiblesweep, shouldsweep is true and forced/forcesweep are false, and sweeping is in progress. Now a forcesweep request comes in: sets forcesweep to true, performs a notify (which does nothing) and then does a wait(1000). The sweeper thread completes the sweep, sees forced==false and so doesn't do a notify, and itself goes back to waiting until it either times out (after 24 hours!) or is notified again (when codecache fills up enough). The forcesweep caller will keep looping checking forcedsweep, which is still true, and doing a wait for 1 second until the next actual sweep occurs.

I overlooked the fact that when it wakes up after 1 second it will notify the sweeper thread again. So no problem. Thanks for pointing that out Tobias.

David

I think you need the sweeperthread to check forcedsweep before waiting ie:

251 void NMethodSweeper::sweeperloop() { 252 bool timeout; 253 while (true) { 254 { 255 ThreadBlockInVM tbivm(JavaThread::current()); 256 MutexLockerEx waiter(CodeCachelock, Mutex::nosafepointcheckflag); + if (!forcedsweep) { 257 const long waittime = 606024 * 1000; 258 timeout = CodeCachelock->wait(Mutex::nosafepointcheckflag, waittime); + } 259 } ! 260 if (!timeout || forcedsweep) { 261 possiblysweep(); 262 } 263 } 264 } That would trigger two back-to-back sweeps. Alternatively, if two back-to-backs sweeps is not ideal then in possiblysweep you'd need to unconditionally grab the lock and check forcedsweep and clear it and notify. But you must check forcedsweep with the lock hold. Also under the current approach concurrent calls to forcesweep wouldn't force a sweep per call, but would simply block until A forced sweep had occurred (or just a sweep depending on what you do above). That's probably what you want but I wanted to flag it just in case. Cheers, David

I executed 1k runs of the failing test.

Thanks, Tobias

I looked in reviews of original changes for 8064669 and we did not ask about that.

Thanks, Vladimir On 3/25/15 6:53 AM, Tobias Hartmann wrote: Hi,

please review the following patch. https://bugs.openjdk.java.net/browse/JDK-8075214 http://cr.openjdk.java.net/~thartmann/8075214/webrev.00/ Problem: The test uses the Whitebox API to enforce sweeping by creating and starting a 'CodeCacheSweeperThread'. During creation of the thread, the interpreter crashes in j.l.ThreadGroup.add(Thread t) [1] while executing a subtype check to validate that 't' is a subtype of j.l.Thread [2]. The problem is that we pass 'JavaThread->threadObj()' to 'ThreadGroup.add' which is invalid due to a GC that moved the object. The GC does not know about the thread because it was not yet added to the threads list and therefore does not update the oop. Solution: Instead of calling 'JavaThread::allocatethreadObj', the initialization is moved to the caller to make sure that setting the thread oop is done together with adding the thread to the threads list. I also fixed the missing oom handling described as one of the problems in JDK-8072377 [3]. Testing: - 1k runs of failing testcase - JPRT Thanks, Tobias [1] http://hg.openjdk.java.net/jdk9/hs-comp/jdk/file/tip/src/java.base/share/classes/java/lang/ThreadGroup.java#l896 [2] see ' gensubtypecheck' in 'TemplateTable::aastore'_ [3] https://bugs.openjdk.java.net/browse/JDK-8072377

More information about the hotspot-compiler-dev mailing list