Support JRuby and fix travis failures #108
Conversation
Free ptr objects in ensure blocks and add workaround for lack of read(:pointer), read_string_to_null and read_int64 ffi methods in JRuby Add JRuby 9.2.9.0 to travis WIP: Debugging. Modify error_spec and skip the spec using fork in JRuby. Use RbConfig to check for MacOS. WIP: Free @native_kafka in the consumer/producer during close Manually free the AutoPointer using ruby instead of a C call Switch to confluence docker images ProducerSpec: Closing the IO in forked process Avoiding AutoPointer instances being copied into forked process Alter the consumer spec to test for the message produced Remove consumer commit after assertions and increase the handle wait for JRuby for reason still unknown Rely on GC to call the finalizer to close @native_kafka socket Revert "Rely on GC to call the finalizer to close @native_kafka socket" This reverts commit 38ea6d6. Skip AutoPointer for consumer class Remove rd_kafka_consumer_close as rd_kafka_destroy takes care of it for consumers Adding rd_kafka_consumer_close and ensure it is only called once Skip fork spec Revert "Skip fork spec" This reverts commit d4cfb58. Remove autorelease and free pointers accessed using read_pointer
When converting a Rdkafka::TopicPartitionList to a native type, Rdkafka was depending on AutoPointer to eventually free them. There are a couple cases where a Client handle returns a pointer to the application that the application is then required to free, which could be leading to test instability. This takes a pass at ensuring all uses of native TopicPartitionList instances (even those return) are deterministically freed. Ensure poll returns nil after a consumer is closed Add the read_array_of_uint64 workaround for JRuby
…stroy will result in a segmentation fault but not #free
|
Just as a note, it looks like jruby is working on updating their version of FFI for the new APIs. |
…4_t. Use AutoPointer#free in producer instead of rd_kafka_destroy to close @native_kafka instances if consumer#close is not called.
|
Even though this build is green, there is a segmentation fault in the forked process. The |
|
Looking very good, thanks for your hard work on this! Guess we should get rid of the last usage of autopointer. |
|
Debug level shows this before @thijsc Looks like this is similar to confluentinc/confluent-kafka-dotnet#1129 and the reason is this. |
|
Looks like travis encountered a segmentation fault while doing a cleanup after running the specs even after getting rid of We might need debugging enabled in Travis as the specs work locally for me against JRuby delivery callback spec failure is intermittent in travis and always passes in local. |
|
@thijsc 6d7b472 ensures that all socket connections are closed which fixed the travis segmentation fault. Twelve socket connections were previously open by the time I modified the spec in 3e5331a to make sure the producer is instantiated in the child process. I think sharing the sockets between processes should not be encouraged. It's easier to create a new producer in a forked process. I changed the behavior of |
…calls the delivery callback
|
Thanks so much for your hard work on this! |
|
Seems that you have not updated |
|
@rearden-steel I'm assuming that it'll be done by the maintainers when publishing a new release. |
Re-opening #106 here to eliminate the noise due to numerous debugging commits.
I find it easier to trigger travis builds than running the specs locally.
The changes in this PR also include @gaffneyc's changes in #107:
read_int64,read(:pointer),read_string_to_nullin JRuby.Rdkafka::Consumerand explicitly callingrdkafka_destroyduring close.rd_kafka_consumer_pollafter consumer is closed.RbConfig::CONFIGto check the host OS which is JRuby friendly..travis.yml.AutoPointeras it'll lead to closing the socket connections when a child process created usingKernel#forkexits which could lead to segmentation faults.Findings:
E, [2020-01-29T06:17:58.952506 #9416] ERROR -- : rdkafka: [thrd:GroupCoordinator]: 1/1 brokers are downin specs is trivial as per confluentinc/confluent-kafka-dotnet#1129 (comment)I'll gladly rebase this PR if you'd wish to merge #107 first.