I disagree with using `ReferenceCountUtil.safeRelease()` everywhere.
The implementation is throwing any Throwable:

```java

public static void safeRelease(Object msg) {
    try {
        release(msg);
    } catch (Throwable var2) {
        logger.warn("Failed to release a message: {}", msg, var2);
    }
}
```

When `release` throws an exception, it means the logic is wrong. For
example, you have released a ByteBuf whose refcnt is 1 twice. We
should make it clear where fast-fail is not allowed and catch the
exception in these places.

Thanks,
Yunze

On Sun, Jan 29, 2023 at 7:57 PM steven lu <lushiji2...@gmail.com> wrote:
>
> [DISCUSS] PIP-244: Refactor ByteBuf release method
> Hello everyone. I hope you guys are all doing well. I would like to start
> the discussion for PIP-244 https://github.com/apache/pulsar/issues/19350,
> Please let me know if you have any concerns or questions.
> ------- Paste original PIP content to help quote ------
>
> ### Motivation
>
> It may throw an exception when release a `ByteBuf` object. so the exception
> in `ByteBuf.release` should be checked.
>
> ### Goal
>
> Use `ReferenceCountUtil.safeRelease()` instead of `ByteBuf.release()` in
> all Pulsar's classes.
>
> ### API Changes
>
> _No response_
>
> ### Implementation
>
> 1. Use `ReferenceCountUtil.safeRelease()` instead of `ByteBuf.release()`.

Reply via email to