Blizzard / Blizzard/node-rdkafka
Possible persistent handle leak when producing with `opaque` and no delivery callback
- 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
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