+1 I created this issue:
https://bugs.openjdk.java.net/browse/JDK-8208362 The suggested fix requires a tweak though since an instance of a DirectBuffer interface is passed. This is required because views over direct ByteBuffers can be created. Florian, i will push tomorrow and assign you as at the contributor. Paul. diff -r 448cd909c9e2 src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template --- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template Thu Jul 26 11:53:59 2018 -0700 +++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template Thu Jul 26 16:46:20 2018 -0700 @@ -194,7 +194,8 @@ #if[byte] cleaner = null; #end[byte] - att = db; + Object attachment = db.attachment(); + att = (attachment == null ? db : attachment); #else[rw] super(db, mark, pos, lim, cap, off); this.isReadOnly = true; > On Jul 20, 2018, at 11:15 AM, mark.reinh...@oracle.com wrote: > > 2018/7/20 10:55:52 -0700, Florian Weimer <f...@deneb.enyo.de>: >> ... >> >> The constructor invoked: >> >> | // For duplicates and slices >> | // >> | Direct$Type$Buffer$RW$$BO$(DirectBuffer db, // package-private >> | int mark, int pos, int lim, int cap, >> | int off) >> | { >> | #if[rw] >> | super(mark, pos, lim, cap); >> | address = db.address() + off; >> | #if[byte] >> | cleaner = null; >> | #end[byte] >> | att = db; >> | #else[rw] >> | super(db, mark, pos, lim, cap, off); >> | this.isReadOnly = true; >> | #end[rw] >> | } >> >> The key part is the assignment to the att member. If I understand >> this correctly, it is needed to keep the backing object alive during >> the lifetime of this buffer. > > Correct. > >> However, it causes the creation of a >> long chain of buffer objects. With -Xmx100m or so, the following test >> will OOM fairly quickly for this reason: >> >> | volatile ByteBuffer buffer; >> | … >> | buffer = ByteBuffer.allocateDirect(16384); >> | while (true) { >> | buffer = buffer.duplicate(); >> | } > > Well spotted! This bug has been lurking there for sixteen years. > >> >> I wonder if it would be possible to change the setting of the att >> member to this instead: >> >> | if (db.att == null) { >> | att = db; >> | } else { >> | att = db.att; >> | } >> >> This would only keep the object alive which actually owns the backing >> storage, as if Buffer::slice had been invoked on it directly. > > Your suggested fix looks fine. > > - Mark
