Long chains created by direct Buffer::slice

[mark.reinhold at oracle.com]

2018/7/20 10:55:52 -0700, Florian Weimer <fw at 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