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
