I think that we must do something, current master branch is not stable.

My colleague Massimiliano opened this issue
https://github.com/apache/bookkeeper/issues/3751

Basically in the current master there is some problem that leads to
Netty BytBuf corruption.

The problem is solved by reverting this PR
https://github.com/apache/bookkeeper/pull/3528 Fix memory leak when
reading entry but the connection disconnected

Enrico

Il giorno lun 30 gen 2023 alle ore 08:04 steven lu
<lushiji2...@gmail.com> ha scritto:
>
> There are three ways to write release in current pulsar and bookkeeper
> projects:
> 1.ByteBuf.release: Call release directly without handling any exceptions;
> 2.ReferenceCountUtil.release: return processing status;
> 3. ReferenceCountUtil.safeRelease: without return value, print exception
> information
>
> I don't think it is necessary to revert these PRs. We can discuss what is
> the correct way to release, and then modify these three ways into the
> correct way.
>
> Hang Chen <chenh...@apache.org> 于2023年1月30日周一 12:28写道:
>
> > Hi guys,
> >    In BP-59, which was not discussed in the dev mail list and without
> > a vote, refactored the ByteBuf release method by
> > ReferenceCountUtil.safeRelease() instead of ByteBuf.release().
> >
> > In the `ReferenceCountUtil.safeRelease()`, it catches the release
> > exception. This change can make the ByteBuf release without any
> > exceptions and makes the code run well, but it will make bugs hide
> > deeper and more challenging to find out.
> >    ```java
> >    public static void safeRelease(Object msg) {
> >         try {
> >             release(msg);
> >         } catch (Throwable t) {
> >             logger.warn("Failed to release a message: {}", msg, t);
> >         }
> >     }
> >    ```
> > For example, if there is a bug to release the ByteBuf twice, whose
> > refcnt is 1, we can find out the bug quickly if we use
> > ByteBuf.release(), but the bug will be hard to find out if we use
> > `ReferenceCountUtil.safeRelease()`
> >
> > There are 12 PRs to refactor the ByteBuf release method, and I suggest
> > reverting those PRs. Do you have any ideas?
> >
> > https://github.com/apache/bookkeeper/pull/3673
> > https://github.com/apache/bookkeeper/pull/3674
> > https://github.com/apache/bookkeeper/pull/3687
> > https://github.com/apache/bookkeeper/pull/3688
> > https://github.com/apache/bookkeeper/pull/3689
> > https://github.com/apache/bookkeeper/pull/3691
> > https://github.com/apache/bookkeeper/pull/3693
> > https://github.com/apache/bookkeeper/pull/3694
> > https://github.com/apache/bookkeeper/pull/3695
> > https://github.com/apache/bookkeeper/pull/3698
> > https://github.com/apache/bookkeeper/pull/3700
> > https://github.com/apache/bookkeeper/pull/3703
> >
> >
> > Thanks,
> > Hang
> >

Reply via email to