Blizzard / Blizzard/node-rdkafka

Possible persistent handle leak when producing with `opaque` and no delivery callback

Open
#1,147 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
2.2k
Forks
403
PR merge metrics
No merged PRs in 30d

Description

# Possible persistent handle leak when producing with `opaque` and no delivery callback

I found a possible native and JavaScript-object retention leak in `produce()` when an `opaque` value is passed without a delivery callback.

Files: `lib/producer.js`, `src/producer.cc`, `src/callbacks.cc`

Functions: `Producer.prototype.produce`, `Producer::NodeProduce`, `Delivery::dr_cb`, `DeliveryReportDispatcher::Flush`

Relevant public API:

```js
Producer.prototype.produce = function(topic, partition, message, key, timestamp, opaque, headers) {
// ...
return this._errorWrap(
this._client.produce(topic, partition, message, key, timestamp, opaque, headers));
};
```

Delivery callbacks are only registered when `dr_cb` or `dr_msg_cb` is configured:

```js
if (dr_msg_cb || dr_cb) {
this._cb_configs.event.delivery_cb = function(err, report) {
this.emit('delivery-report', err, report);
}.bind(this);
}
```

The native `produce()` implementation allocates a persistent handle for every
defined `opaque` argument:

```cpp
void* opaque = NULL;
if (info.Length() > 5 && !info[5]->IsUndefined()) {
opaque = new Nan::Persistent(info[5]);
}
```

The synchronous enqueue-error path cleans it up:

```cpp
if (error_code != 0 && opaque) {
Nan::Persistent *persistent =
static_cast *>(opaque);
persistent->Reset();
delete persistent;
}
```

The successful delivery cleanup happens later in `Flush()`:

```cpp
if (event.opaque) {
Nan::Persistent *persistent =
static_cast *>(event.opaque);
v8::Local object = Nan::New(*persistent);
Nan::Set(jsobj, Nan::New("opaque").ToLocalChecked(), object);

persistent->Reset();
delete persistent;
}
```

But the delivery callback returns before creating a `DeliveryReport` when no JS
delivery callback is registered:

```cpp
void Delivery::dr_cb(RdKafka::Message &message) {
if (!dispatcher.HasCallbacks()) {
return;
}

DeliveryReport msg(message, m_dr_msg_cb);
// ...
}
```

So a successfully enqueued message with `opaque` can bypass both cleanup paths:
it was accepted by librdkafka, but no delivery report is queued to release the
`Nan::Persistent`.

Suggested fix: reclaim `message.msg_opaque()` even when there are no JS delivery
callbacks, preferably on the Node/V8 thread. Another option is to avoid
allocating the persistent when no delivery callback path can ever expose it.

---

Related: #953 (closed) reported the sibling case — `opaque` leaking when
`produce()` itself errors. That synchronous enqueue-error path is now cleaned up
(the `if (error_code != 0 && opaque) { ...delete... }` block above). This report
covers the remaining path: a message that is **successfully enqueued while no JS
delivery callback is registered**, where `Delivery::dr_cb` early-returns before a
`DeliveryReport` is built, so `Flush()` never releases the persistent.

Contributor guide

Open the contributing guide

Research direction

Start by tracing opaque ownership through lib/producer.js, src/producer.cc, and src/callbacks.cc, focusing on Producer.prototype.produce, Producer::NodeProduce, Delivery::dr_cb, and DeliveryReportDispatcher::Flush. Confirm the successful-enqueue path when no delivery callback is configured, then verify that the persistent handle is reclaimed without changing delivery-report behavior when callbacks are present.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, javascript, nodejs
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.