haskell-works / hw-kafka-client

Kafka client for Haskell, including auto-rebalancing consumers
MIT License
139 stars 50 forks source link

Statistics callback FFI type wrong #165

Closed phile314 closed 3 years ago

phile314 commented 3 years ago

According to the rdkafka doc, the rd_kafka_conf_set_stats_cb is defined as follows:

RD_EXPORT void rd_kafka_conf_set_stats_cb  (rd_kafka_conf_t * conf, int(*)(rd_kafka_t *rk, char *json, size_t json_len, void *opaque) stats_cb ) 

Note that the return type of the passed function is int:

If the application wishes to hold on to the json pointer and free it at a later time it must return 1 from the stats_cb. If the application returns 0 from the stats_cb then librdkafka will immediately free the json pointer.

Source: https://docs.confluent.io/3.3.1/clients/librdkafka/rdkafka_8h.html#a597d00432e3ca22174d18e7e348fb766

In hw-kafka-client, the FFI definition is as follows:

---- Stats Callback
type StatsCallback' = Ptr RdKafkaT -> CString -> CSize -> Word8Ptr -> IO ()
....
foreign import ccall safe "rd_kafka.h rd_kafka_conf_set_stats_cb"
    rdKafkaConfSetStatsCb' :: Ptr RdKafkaConfT -> FunPtr StatsCallback' -> IO ()

I believe instead it should be:

type StatsCallback' = Ptr RdKafkaT -> CString -> CSize -> Word8Ptr -> IO CInt

And I think we want to always return zero, as the callback copies the json into a new bytestring.

I have no idea what the implications of this bug are, maybe it is not a problem in practice, maybe it is...

I will make a PR next week or so.

phile314 commented 3 years ago

It seems to also affect the SocketCallback.

AlexeyRaga commented 3 years ago

Makes sense, thanks!