From c0cb6fbff52ffff27cbe9e17a85604dd768debbd Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 27 Feb 2024 10:15:06 -0800 Subject: [PATCH 01/25] Add Rust connection logic and example --- go/.gitignore | 3 + go/Cargo.toml | 2 + go/examples/main.go | 32 ++++ go/glide/glide.go | 137 ++++++++++++++ go/glide/lib.h | 145 ++++++++++++++ go/src/lib.rs | 453 +++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 769 insertions(+), 3 deletions(-) create mode 100644 go/examples/main.go create mode 100644 go/glide/lib.h diff --git a/go/.gitignore b/go/.gitignore index 5b5c1537a7..12afb3251d 100644 --- a/go/.gitignore +++ b/go/.gitignore @@ -1,4 +1,7 @@ # Go compilation files *.pb.go +# Examples +examples/examples + reports diff --git a/go/Cargo.toml b/go/Cargo.toml index 8bbc3b5b12..7a43c226d3 100644 --- a/go/Cargo.toml +++ b/go/Cargo.toml @@ -13,6 +13,8 @@ redis = { path = "../submodules/redis-rs/redis", features = ["aio", "tokio-comp" glide-core = { path = "../glide-core" } tokio = { version = "^1", features = ["rt", "macros", "rt-multi-thread", "time"] } protobuf = { version = "3.3.0", features = [] } +logger_core = {path = "../logger_core"} +tracing-subscriber = "0.3.16" [profile.release] lto = true diff --git a/go/examples/main.go b/go/examples/main.go new file mode 100644 index 0000000000..9502f32006 --- /dev/null +++ b/go/examples/main.go @@ -0,0 +1,32 @@ +package main + +import ( + "fmt" + "github.com/aws/glide-for-redis/go/glide/glide" +) + +func main() { + fmt.Println("Starting go-glide client...") + client := glide.GlideRedisClient{} + + err := client.ConnectToRedis("localhost", 6379, false, false) + if err != nil { + return + } + + err = client.Set("FOO", "BAR") + if err != nil { + panic(err) + } + fmt.Println("SET FOO : BAR") + + val, err := client.Get("FOO") + if err != nil { + panic(err) + } + fmt.Println("GET FOO :", val) + + client.CloseClient() + + fmt.Println("Disconnected from Redis") +} diff --git a/go/glide/glide.go b/go/glide/glide.go index 767864d035..5483c95a26 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -1 +1,138 @@ package glide + +/* +#cgo LDFLAGS: -L../target/release -lglide_rs +#include "lib.h" + +void successCallback(uintptr_t channelPtr, char *message); +void failureCallback(uintptr_t channelPtr, char *errMessage); +*/ +import "C" + +import ( + "fmt" + "unsafe" +) + +type GlideRedisClient struct { + coreClient unsafe.Pointer +} + +type payload struct { + value string + errMessage error +} + +//export successCallback +func successCallback(channelPtr C.uintptr_t, message *C.char) { + goMessage := C.GoString(message) + goChannelPointer := uintptr(channelPtr) + resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) + resultChannel <- payload{value: goMessage, errMessage: nil} +} + +//export failureCallback +func failureCallback(channelPtr C.uintptr_t, errMessage *C.char) { + goMessage := C.GoString(errMessage) + goChannelPointer := uintptr(channelPtr) + resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) + resultChannel <- payload{value: "", errMessage: fmt.Errorf("error at redis operation: %s", goMessage)} +} + +func (glideRedisClient *GlideRedisClient) ConnectToRedis(host string, port int, useSSL bool, clusterModeEnabled bool) error { + caddress := C.CString(host) + defer C.free(unsafe.Pointer(caddress)) + + response := (C.struct_ConnectionResponse)(C.create_client(caddress, C.uint32_t(port), C._Bool(useSSL), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) + glideRedisClient.coreClient = response.conn_ptr + if glideRedisClient.coreClient == nil { + return fmt.Errorf("error connecting to glideRedisClient") + } + return nil +} + +func (glideRedisClient *GlideRedisClient) Set(key string, value interface{}) error { + strValue := fmt.Sprintf("%v", value) + ckey := C.CString(key) + cval := C.CString(strValue) + defer C.free(unsafe.Pointer(ckey)) + defer C.free(unsafe.Pointer(cval)) + + resultChannel := make(chan payload) + resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) + + args := []string{key, strValue} + argv := make([]*C.char, len(args)) + for index, string := range args { + cString := C.CString(string) + defer C.free(unsafe.Pointer(cString)) + argv[index] = cString + } + requestType := C.uint32_t(3) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(2), &argv[0]) + + resultPayload := <-resultChannel + + return resultPayload.errMessage +} + +func (glideRedisClient *GlideRedisClient) Get(key string) (string, error) { + ckey := C.CString(key) + defer C.free(unsafe.Pointer(ckey)) + + resultChannel := make(chan payload) + resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) + + args := []string{key} + argv := make([]*C.char, len(args)) + for index, string := range args { + cString := C.CString(string) + defer C.free(unsafe.Pointer(cString)) + argv[index] = cString + } + requestType := C.uint32_t(2) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(1), &argv[0]) + resultPayload := <-resultChannel + + return resultPayload.value, nil +} + +func (glideRedisClient *GlideRedisClient) Ping() (string, error) { + resultChannel := make(chan payload) + resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) + + args := []string{} + argv := make([]*C.char, len(args)) + for index, string := range args { + cString := C.CString(string) + defer C.free(unsafe.Pointer(cString)) + argv[index] = cString + } + requestType := C.uint32_t(4) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &argv[0]) + resultPayload := <-resultChannel + + return resultPayload.value, resultPayload.errMessage +} + +func (glideRedisClient *GlideRedisClient) Info() (string, error) { + resultChannel := make(chan payload) + resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) + + args := []string{} + argv := make([]*C.char, len(args)) + for index, string := range args { + cString := C.CString(string) + defer C.free(unsafe.Pointer(cString)) + argv[index] = cString + } + requestType := C.uint32_t(5) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &argv[0]) + resultPayload := <-resultChannel + + return resultPayload.value, resultPayload.errMessage +} + +func (glideRedisClient *GlideRedisClient) CloseClient() { + C.close_client(glideRedisClient.coreClient) +} diff --git a/go/glide/lib.h b/go/glide/lib.h new file mode 100644 index 0000000000..ccbb07a4e8 --- /dev/null +++ b/go/glide/lib.h @@ -0,0 +1,145 @@ +#include +#include +#include +#include + +enum ErrorType { + ClosingError = 0, + RequestError = 1, + TimeoutError = 2, + ExecAbortError = 3, + ConnectionError = 4, +}; +typedef uint32_t ErrorType; + +enum RequestType { + CustomCommand = 1, + GetString = 2, + SetString = 3, + Ping = 4, + Info = 5, + Del = 6, + Select = 7, + ConfigGet = 8, + ConfigSet = 9, + ConfigResetStat = 10, + ConfigRewrite = 11, + ClientGetName = 12, + ClientGetRedir = 13, + ClientId = 14, + ClientInfo = 15, + ClientKill = 16, + ClientList = 17, + ClientNoEvict = 18, + ClientNoTouch = 19, + ClientPause = 20, + ClientReply = 21, + ClientSetInfo = 22, + ClientSetName = 23, + ClientUnblock = 24, + ClientUnpause = 25, + Expire = 26, + HashSet = 27, + HashGet = 28, + HashDel = 29, + HashExists = 30, + MGet = 31, + MSet = 32, + Incr = 33, + IncrBy = 34, + Decr = 35, + IncrByFloat = 36, + DecrBy = 37, + HashGetAll = 38, + HashMSet = 39, + HashMGet = 40, + HashIncrBy = 41, + HashIncrByFloat = 42, + LPush = 43, + LPop = 44, + RPush = 45, + RPop = 46, + LLen = 47, + LRem = 48, + LRange = 49, + LTrim = 50, + SAdd = 51, + SRem = 52, + SMembers = 53, + SCard = 54, + PExpireAt = 55, + PExpire = 56, + ExpireAt = 57, + Exists = 58, + Unlink = 59, + TTL = 60, + Zadd = 61, + Zrem = 62, + Zrange = 63, + Zcard = 64, + Zcount = 65, + ZIncrBy = 66, + ZScore = 67, + Type = 68, + HLen = 69, + Echo = 70, + ZPopMin = 71, + Strlen = 72, + Lindex = 73, + ZPopMax = 74, + XRead = 75, + XAdd = 76, + XReadGroup = 77, + XAck = 78, + XTrim = 79, + XGroupCreate = 80, + XGroupDestroy = 81, +}; +typedef uint32_t RequestType; + +typedef struct Level Level; + +typedef struct Option_Level Option_Level; + +typedef struct RedisErrorFFI { + const char *message; + ErrorType error_type; +} RedisErrorFFI; + +typedef struct ConnectionResponse { + const void *conn_ptr; + const struct RedisErrorFFI *error; +} ConnectionResponse; + +typedef void (*SuccessCallback)(uintptr_t channel_address, const char *message); + +typedef void (*FailureCallback)(uintptr_t channel_address, const char *err_message); + +/** + * Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. + */ +struct ConnectionResponse create_client(const char *host, + uint32_t port, + bool use_tls, + SuccessCallback success_callback, + FailureCallback failure_callback); + +void close_client(const void *client_ptr); + +void command(const void *client_ptr, + uintptr_t channel, + RequestType command_type, + uintptr_t arg_count, + const char *const *args); + +/** + * # Safety + * Unsafe function because creating string from pointer + */ +void log(struct Level log_level, const char *log_identifier, const char *message); + +/** + * # Safety + * Unsafe function because creating string from pointer + */ +struct Level init(struct Option_Level level, const char *file_name); diff --git a/go/src/lib.rs b/go/src/lib.rs index 26b1dacc35..15dccc7715 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -1,9 +1,456 @@ /** * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ -use std::ffi::c_void; +use glide_core::connection_request; +use glide_core::{client::Client as GlideClient, connection_request::NodeAddress}; +use redis::{cmd, Cmd, FromRedisValue, RedisResult, Value}; +use std::{ + ffi::{c_void, CStr, CString}, + os::raw::c_char, +}; +use tokio::runtime::Builder; +use tokio::runtime::Runtime; + +pub type SuccessCallback = + unsafe extern "C" fn(channel_address: usize, message: *const c_char) -> (); +pub type FailureCallback = + unsafe extern "C" fn(channel_address: usize, err_message: *const c_char) -> (); + +pub enum Level { + Error = 0, + Warn = 1, + Info = 2, + Debug = 3, + Trace = 4, +} + +#[repr(C)] +pub struct ConnectionResponse { + conn_ptr: *const c_void, + error: *const RedisErrorFFI, +} + +#[repr(C)] +pub struct RedisErrorFFI { + message: *const c_char, + error_type: ErrorType, +} + +#[repr(u32)] +pub enum ErrorType { + ClosingError = 0, + RequestError = 1, + TimeoutError = 2, + ExecAbortError = 3, + ConnectionError = 4, +} + +pub struct Client { + client: GlideClient, + success_callback: SuccessCallback, + failure_callback: FailureCallback, // TODO - add specific error codes + runtime: Runtime, +} + +fn create_connection_request( + host: String, + port: u32, + use_tls: bool, +) -> connection_request::ConnectionRequest { + let mut address_info = NodeAddress::new(); + address_info.host = host.to_string().into(); + address_info.port = port; + let addresses_info = vec![address_info]; + let mut connection_request = connection_request::ConnectionRequest::new(); + connection_request.addresses = addresses_info; + connection_request.tls_mode = if use_tls { + connection_request::TlsMode::SecureTls + } else { + connection_request::TlsMode::NoTls + } + .into(); + + connection_request +} + +fn create_client_internal( + host: *const c_char, + port: u32, + use_tls: bool, + success_callback: SuccessCallback, + failure_callback: FailureCallback, +) -> RedisResult { + let host_cstring = unsafe { CStr::from_ptr(host as *mut c_char) }; + let host_string = host_cstring.to_str()?.to_string(); + let request = create_connection_request(host_string, port, use_tls); + let runtime = Builder::new_multi_thread() + .enable_all() + .thread_name("GLIDE for Redis Go thread") + .build()?; + let _runtime_handle = runtime.enter(); + let client = runtime.block_on(GlideClient::new(request)).unwrap(); // TODO - handle errors. + Ok(Client { + client, + success_callback, + failure_callback, + runtime, + }) +} + +/// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. +#[no_mangle] +pub extern "C" fn create_client( + host: *const c_char, + port: u32, + use_tls: bool, + success_callback: SuccessCallback, + failure_callback: FailureCallback, +) -> ConnectionResponse { + match create_client_internal(host, port, use_tls, success_callback, failure_callback) { + Err(err) => { + let message_cstring = CString::new(err.to_string()).unwrap(); + ConnectionResponse { + conn_ptr: std::ptr::null(), + error: &RedisErrorFFI { + message: message_cstring.as_ptr(), + error_type: ErrorType::ConnectionError, + }, + } + } + Ok(client) => ConnectionResponse { + conn_ptr: Box::into_raw(Box::new(client)) as *const c_void, + error: std::ptr::null(), + }, + } +} #[no_mangle] -pub extern "C" fn create_connection() -> *const c_void { - todo!() +pub extern "C" fn close_client(client_ptr: *const c_void) { + let client_ptr = unsafe { Box::from_raw(client_ptr as *mut Client) }; + let _runtime_handle = client_ptr.runtime.enter(); + drop(client_ptr); +} + +// Cannot use glide_core::redis_request::RequestType, because it is not FFI safe +#[repr(u32)] +pub enum RequestType { + // copied from redis_request.proto + CustomCommand = 1, + GetString = 2, + SetString = 3, + Ping = 4, + Info = 5, + Del = 6, + Select = 7, + ConfigGet = 8, + ConfigSet = 9, + ConfigResetStat = 10, + ConfigRewrite = 11, + ClientGetName = 12, + ClientGetRedir = 13, + ClientId = 14, + ClientInfo = 15, + ClientKill = 16, + ClientList = 17, + ClientNoEvict = 18, + ClientNoTouch = 19, + ClientPause = 20, + ClientReply = 21, + ClientSetInfo = 22, + ClientSetName = 23, + ClientUnblock = 24, + ClientUnpause = 25, + Expire = 26, + HashSet = 27, + HashGet = 28, + HashDel = 29, + HashExists = 30, + MGet = 31, + MSet = 32, + Incr = 33, + IncrBy = 34, + Decr = 35, + IncrByFloat = 36, + DecrBy = 37, + HashGetAll = 38, + HashMSet = 39, + HashMGet = 40, + HashIncrBy = 41, + HashIncrByFloat = 42, + LPush = 43, + LPop = 44, + RPush = 45, + RPop = 46, + LLen = 47, + LRem = 48, + LRange = 49, + LTrim = 50, + SAdd = 51, + SRem = 52, + SMembers = 53, + SCard = 54, + PExpireAt = 55, + PExpire = 56, + ExpireAt = 57, + Exists = 58, + Unlink = 59, + TTL = 60, + Zadd = 61, + Zrem = 62, + Zrange = 63, + Zcard = 64, + Zcount = 65, + ZIncrBy = 66, + ZScore = 67, + Type = 68, + HLen = 69, + Echo = 70, + ZPopMin = 71, + Strlen = 72, + Lindex = 73, + ZPopMax = 74, + XRead = 75, + XAdd = 76, + XReadGroup = 77, + XAck = 78, + XTrim = 79, + XGroupCreate = 80, + XGroupDestroy = 81, +} + +// copied from glide_core::socket_listener::get_command +fn get_command(request_type: RequestType) -> Option { + match request_type { + //RequestType::InvalidRequest => None, + RequestType::CustomCommand => Some(Cmd::new()), + RequestType::GetString => Some(cmd("GET")), + RequestType::SetString => Some(cmd("SET")), + RequestType::Ping => Some(cmd("PING")), + RequestType::Info => Some(cmd("INFO")), + RequestType::Del => Some(cmd("DEL")), + RequestType::Select => Some(cmd("SELECT")), + RequestType::ConfigGet => Some(get_two_word_command("CONFIG", "GET")), + RequestType::ConfigSet => Some(get_two_word_command("CONFIG", "SET")), + RequestType::ConfigResetStat => Some(get_two_word_command("CONFIG", "RESETSTAT")), + RequestType::ConfigRewrite => Some(get_two_word_command("CONFIG", "REWRITE")), + RequestType::ClientGetName => Some(get_two_word_command("CLIENT", "GETNAME")), + RequestType::ClientGetRedir => Some(get_two_word_command("CLIENT", "GETREDIR")), + RequestType::ClientId => Some(get_two_word_command("CLIENT", "ID")), + RequestType::ClientInfo => Some(get_two_word_command("CLIENT", "INFO")), + RequestType::ClientKill => Some(get_two_word_command("CLIENT", "KILL")), + RequestType::ClientList => Some(get_two_word_command("CLIENT", "LIST")), + RequestType::ClientNoEvict => Some(get_two_word_command("CLIENT", "NO-EVICT")), + RequestType::ClientNoTouch => Some(get_two_word_command("CLIENT", "NO-TOUCH")), + RequestType::ClientPause => Some(get_two_word_command("CLIENT", "PAUSE")), + RequestType::ClientReply => Some(get_two_word_command("CLIENT", "REPLY")), + RequestType::ClientSetInfo => Some(get_two_word_command("CLIENT", "SETINFO")), + RequestType::ClientSetName => Some(get_two_word_command("CLIENT", "SETNAME")), + RequestType::ClientUnblock => Some(get_two_word_command("CLIENT", "UNBLOCK")), + RequestType::ClientUnpause => Some(get_two_word_command("CLIENT", "UNPAUSE")), + RequestType::Expire => Some(cmd("EXPIRE")), + RequestType::HashSet => Some(cmd("HSET")), + RequestType::HashGet => Some(cmd("HGET")), + RequestType::HashDel => Some(cmd("HDEL")), + RequestType::HashExists => Some(cmd("HEXISTS")), + RequestType::MSet => Some(cmd("MSET")), + RequestType::MGet => Some(cmd("MGET")), + RequestType::Incr => Some(cmd("INCR")), + RequestType::IncrBy => Some(cmd("INCRBY")), + RequestType::IncrByFloat => Some(cmd("INCRBYFLOAT")), + RequestType::Decr => Some(cmd("DECR")), + RequestType::DecrBy => Some(cmd("DECRBY")), + RequestType::HashGetAll => Some(cmd("HGETALL")), + RequestType::HashMSet => Some(cmd("HMSET")), + RequestType::HashMGet => Some(cmd("HMGET")), + RequestType::HashIncrBy => Some(cmd("HINCRBY")), + RequestType::HashIncrByFloat => Some(cmd("HINCRBYFLOAT")), + RequestType::LPush => Some(cmd("LPUSH")), + RequestType::LPop => Some(cmd("LPOP")), + RequestType::RPush => Some(cmd("RPUSH")), + RequestType::RPop => Some(cmd("RPOP")), + RequestType::LLen => Some(cmd("LLEN")), + RequestType::LRem => Some(cmd("LREM")), + RequestType::LRange => Some(cmd("LRANGE")), + RequestType::LTrim => Some(cmd("LTRIM")), + RequestType::SAdd => Some(cmd("SADD")), + RequestType::SRem => Some(cmd("SREM")), + RequestType::SMembers => Some(cmd("SMEMBERS")), + RequestType::SCard => Some(cmd("SCARD")), + RequestType::PExpireAt => Some(cmd("PEXPIREAT")), + RequestType::PExpire => Some(cmd("PEXPIRE")), + RequestType::ExpireAt => Some(cmd("EXPIREAT")), + RequestType::Exists => Some(cmd("EXISTS")), + RequestType::Unlink => Some(cmd("UNLINK")), + RequestType::TTL => Some(cmd("TTL")), + RequestType::Zadd => Some(cmd("ZADD")), + RequestType::Zrem => Some(cmd("ZREM")), + RequestType::Zrange => Some(cmd("ZRANGE")), + RequestType::Zcard => Some(cmd("ZCARD")), + RequestType::Zcount => Some(cmd("ZCOUNT")), + RequestType::ZIncrBy => Some(cmd("ZINCRBY")), + RequestType::ZScore => Some(cmd("ZSCORE")), + RequestType::Type => Some(cmd("TYPE")), + RequestType::HLen => Some(cmd("HLEN")), + RequestType::Echo => Some(cmd("ECHO")), + RequestType::ZPopMin => Some(cmd("ZPOPMIN")), + RequestType::Strlen => Some(cmd("STRLEN")), + RequestType::Lindex => Some(cmd("LINDEX")), + RequestType::ZPopMax => Some(cmd("ZPOPMAX")), + RequestType::XAck => Some(cmd("XACK")), + RequestType::XAdd => Some(cmd("XADD")), + RequestType::XReadGroup => Some(cmd("XREADGROUP")), + RequestType::XRead => Some(cmd("XREAD")), + RequestType::XGroupCreate => Some(get_two_word_command("XGROUP", "CREATE")), + RequestType::XGroupDestroy => Some(get_two_word_command("XGROUP", "DESTROY")), + RequestType::XTrim => Some(cmd("XTRIM")), + } +} + +// copied from glide_core::socket_listener::get_two_word_command +fn get_two_word_command(first: &str, second: &str) -> Cmd { + let mut cmd = cmd(first); + cmd.arg(second); + cmd +} + +use std::slice::from_raw_parts; +use std::str::Utf8Error; + +pub unsafe fn convert_double_pointer_to_vec( + data: *const *const c_char, + len: usize, +) -> Result, Utf8Error> { + from_raw_parts(data, len) + .iter() + .map(|arg| CStr::from_ptr(*arg).to_str().map(ToString::to_string)) + .collect() +} + +#[no_mangle] +pub unsafe extern "C" fn command( + client_ptr: *const c_void, + channel: usize, + command_type: RequestType, + arg_count: usize, + args: *const *const c_char, +) { + let client = unsafe { Box::leak(Box::from_raw(client_ptr as *mut Client)) }; + // The safety of this needs to be ensured by the calling code. Cannot dispose of the pointer before all operations have completed. + let ptr_address = client_ptr as usize; + + let arg_vec = unsafe { convert_double_pointer_to_vec(args, arg_count) }.unwrap(); // TODO check + + let mut client_clone = client.client.clone(); + client.runtime.spawn(async move { + let mut cmd = get_command(command_type).unwrap(); // TODO check cmd + //print!("{:?}", cmd.args); + cmd.arg(arg_vec); + + let result = client_clone.send_command(&cmd, None).await; + let client = unsafe { Box::leak(Box::from_raw(ptr_address as *mut Client)) }; + let value = match result { + Ok(value) => value, + Err(err) => { + print!(" === err {:?}\n", err); + let c_err_str = CString::new(err.to_string()).expect("CString::new failed"); + unsafe { (client.failure_callback)(channel, c_err_str.as_ptr()) }; + return; + } + }; + + //print!(" === val {:?}\n", value.clone()); + + let result: RedisResult> = match value { + Value::Nil => Ok(None), + Value::Int(num) => Ok(Some(CString::new(format!("{}", num)).unwrap())), + Value::SimpleString(_) | Value::BulkString(_) => { + Option::::from_owned_redis_value(value) + } + Value::Okay => Ok(Some(CString::new("OK").unwrap())), + Value::Double(num) => Ok(Some(CString::new(format!("{}", num)).unwrap())), + Value::Boolean(bool) => Ok(Some(CString::new(format!("{}", bool)).unwrap())), + _ => todo!(), + }; + + //print!(" === result2 {:?}\n", result); + + unsafe { + match result { + Ok(None) => (client.success_callback)(channel, std::ptr::null()), + Ok(Some(c_str)) => (client.success_callback)(channel, c_str.as_ptr()), + Err(err) => { + let c_err_str = CString::new(err.to_string()).expect("CString::new failed"); + (client.failure_callback)(channel, c_err_str.as_ptr()); + } + }; + } + }); +} + +impl From for Level { + fn from(level: logger_core::Level) -> Self { + match level { + logger_core::Level::Error => Level::Error, + logger_core::Level::Warn => Level::Warn, + logger_core::Level::Info => Level::Info, + logger_core::Level::Debug => Level::Debug, + logger_core::Level::Trace => Level::Trace, + } + } +} + +impl From for logger_core::Level { + fn from(level: Level) -> logger_core::Level { + match level { + Level::Error => logger_core::Level::Error, + Level::Warn => logger_core::Level::Warn, + Level::Info => logger_core::Level::Info, + Level::Debug => logger_core::Level::Debug, + Level::Trace => logger_core::Level::Trace, + } + } +} + +#[no_mangle] +#[allow(improper_ctypes_definitions)] +/// # Safety +/// Unsafe function because creating string from pointer +pub unsafe extern "C" fn log( + log_level: Level, + log_identifier: *const c_char, + message: *const c_char, +) { + unsafe { + logger_core::log( + log_level.into(), + CStr::from_ptr(log_identifier) + .to_str() + .expect("Can not read log_identifier argument."), + CStr::from_ptr(message) + .to_str() + .expect("Can not read message argument."), + ); + } +} + +#[no_mangle] +#[allow(improper_ctypes_definitions)] +/// # Safety +/// Unsafe function because creating string from pointer +pub unsafe extern "C" fn init(level: Option, file_name: *const c_char) -> Level { + let file_name_as_str; + unsafe { + file_name_as_str = if file_name.is_null() { + None + } else { + Some( + CStr::from_ptr(file_name) + .to_str() + .expect("Can not read string argument."), + ) + }; + + let logger_level = logger_core::init(level.map(|level| level.into()), file_name_as_str); + logger_level.into() + } } From 22bbf74693ff1cecb23654fe981634c3d3b71115 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 27 Feb 2024 11:45:34 -0800 Subject: [PATCH 02/25] Add error handling logic for connections --- go/glide/glide.go | 283 +++++++++++++++++++++++++++++++--------------- go/glide/lib.h | 2 +- go/src/lib.rs | 2 +- 3 files changed, 192 insertions(+), 95 deletions(-) diff --git a/go/glide/glide.go b/go/glide/glide.go index 5483c95a26..926f9a8551 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -10,129 +10,226 @@ void failureCallback(uintptr_t channelPtr, char *errMessage); import "C" import ( - "fmt" - "unsafe" + "fmt" + "unsafe" ) type GlideRedisClient struct { - coreClient unsafe.Pointer + coreClient unsafe.Pointer } type payload struct { - value string - errMessage error + value string + errMessage error } +type RequestType uint32 + +const ( + _ = iota + CustomCommand RequestType = iota + GetString + SetString + Ping + Info + Del + Select + ConfigGet + ConfigSet + ConfigResetStat + ConfigRewrite + ClientGetName + ClientGetRedir + ClientId + ClientInfo + ClientKill + ClientList + ClientNoEvict + ClientNoTouch + ClientPause + ClientReply + ClientSetInfo + ClientSetName + ClientUnblock + ClientUnpause + Expire + HashSet + HashGet + HashDel + HashExists + MGet + MSet + Incr + IncrBy + Decr + IncrByFloat + DecrBy + HashGetAll + HashMSet + HashMGet + HashIncrBy + HashIncrByFloat + LPush + LPop + RPush + RPop + LLen + LRem + LRange + LTrim + SAdd + SRem + SMembers + SCard + PExpireAt + PExpire + ExpireAt + Exists + Unlink + TTL + Zadd + Zrem + Zrange + Zcard + Zcount + ZIncrBy + ZScore + Type + HLen + Echo + ZPopMin + Strlen + Lindex + ZPopMax + XRead + XAdd + XReadGroup + XAck + XTrim + XGroupCreate + XGroupDestroy +) + +type ErrorType uint32 + +const ( + ClosingError = iota + RequestError + TimeoutError + ExecAbortError + ConnectionError +) + //export successCallback func successCallback(channelPtr C.uintptr_t, message *C.char) { - goMessage := C.GoString(message) - goChannelPointer := uintptr(channelPtr) - resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) - resultChannel <- payload{value: goMessage, errMessage: nil} + goMessage := C.GoString(message) + goChannelPointer := uintptr(channelPtr) + resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) + resultChannel <- payload{value: goMessage, errMessage: nil} } //export failureCallback func failureCallback(channelPtr C.uintptr_t, errMessage *C.char) { - goMessage := C.GoString(errMessage) - goChannelPointer := uintptr(channelPtr) - resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) - resultChannel <- payload{value: "", errMessage: fmt.Errorf("error at redis operation: %s", goMessage)} + goMessage := C.GoString(errMessage) + goChannelPointer := uintptr(channelPtr) + resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) + resultChannel <- payload{value: "", errMessage: fmt.Errorf("error at redis operation: %s", goMessage)} } func (glideRedisClient *GlideRedisClient) ConnectToRedis(host string, port int, useSSL bool, clusterModeEnabled bool) error { - caddress := C.CString(host) - defer C.free(unsafe.Pointer(caddress)) - - response := (C.struct_ConnectionResponse)(C.create_client(caddress, C.uint32_t(port), C._Bool(useSSL), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) - glideRedisClient.coreClient = response.conn_ptr - if glideRedisClient.coreClient == nil { - return fmt.Errorf("error connecting to glideRedisClient") - } - return nil + caddress := C.CString(host) + defer C.free(unsafe.Pointer(caddress)) + + response := (C.struct_ConnectionResponse)(C.create_client(caddress, C.uint32_t(port), C._Bool(useSSL), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) + if response.error != nil { + return fmt.Errorf("Connection error: ", C.GoString(response.error.message)) + } + glideRedisClient.coreClient = response.conn_ptr + return nil } func (glideRedisClient *GlideRedisClient) Set(key string, value interface{}) error { - strValue := fmt.Sprintf("%v", value) - ckey := C.CString(key) - cval := C.CString(strValue) - defer C.free(unsafe.Pointer(ckey)) - defer C.free(unsafe.Pointer(cval)) - - resultChannel := make(chan payload) - resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - - args := []string{key, strValue} - argv := make([]*C.char, len(args)) - for index, string := range args { - cString := C.CString(string) - defer C.free(unsafe.Pointer(cString)) - argv[index] = cString - } - requestType := C.uint32_t(3) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(2), &argv[0]) - - resultPayload := <-resultChannel - - return resultPayload.errMessage + strValue := fmt.Sprintf("%v", value) + ckey := C.CString(key) + cval := C.CString(strValue) + defer C.free(unsafe.Pointer(ckey)) + defer C.free(unsafe.Pointer(cval)) + + resultChannel := make(chan payload) + resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) + + args := []string{key, strValue} + argv := make([]*C.char, len(args)) + for index, string := range args { + cString := C.CString(string) + defer C.free(unsafe.Pointer(cString)) + argv[index] = cString + } + requestType := C.uint32_t(SetString) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(2), &argv[0]) + + resultPayload := <-resultChannel + + return resultPayload.errMessage } func (glideRedisClient *GlideRedisClient) Get(key string) (string, error) { - ckey := C.CString(key) - defer C.free(unsafe.Pointer(ckey)) - - resultChannel := make(chan payload) - resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - - args := []string{key} - argv := make([]*C.char, len(args)) - for index, string := range args { - cString := C.CString(string) - defer C.free(unsafe.Pointer(cString)) - argv[index] = cString - } - requestType := C.uint32_t(2) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(1), &argv[0]) - resultPayload := <-resultChannel - - return resultPayload.value, nil + ckey := C.CString(key) + defer C.free(unsafe.Pointer(ckey)) + + resultChannel := make(chan payload) + resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) + + args := []string{key} + argv := make([]*C.char, len(args)) + for index, string := range args { + cString := C.CString(string) + defer C.free(unsafe.Pointer(cString)) + argv[index] = cString + } + requestType := C.uint32_t(GetString) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(1), &argv[0]) + resultPayload := <-resultChannel + + return resultPayload.value, nil } func (glideRedisClient *GlideRedisClient) Ping() (string, error) { - resultChannel := make(chan payload) - resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - - args := []string{} - argv := make([]*C.char, len(args)) - for index, string := range args { - cString := C.CString(string) - defer C.free(unsafe.Pointer(cString)) - argv[index] = cString - } - requestType := C.uint32_t(4) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &argv[0]) - resultPayload := <-resultChannel - - return resultPayload.value, resultPayload.errMessage + resultChannel := make(chan payload) + resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) + + args := []string{} + argv := make([]*C.char, len(args)) + for index, string := range args { + cString := C.CString(string) + defer C.free(unsafe.Pointer(cString)) + argv[index] = cString + } + requestType := C.uint32_t(Ping) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &argv[0]) + resultPayload := <-resultChannel + + return resultPayload.value, resultPayload.errMessage } func (glideRedisClient *GlideRedisClient) Info() (string, error) { - resultChannel := make(chan payload) - resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - - args := []string{} - argv := make([]*C.char, len(args)) - for index, string := range args { - cString := C.CString(string) - defer C.free(unsafe.Pointer(cString)) - argv[index] = cString - } - requestType := C.uint32_t(5) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &argv[0]) - resultPayload := <-resultChannel - - return resultPayload.value, resultPayload.errMessage + resultChannel := make(chan payload) + resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) + + args := []string{} + argv := make([]*C.char, len(args)) + for index, string := range args { + cString := C.CString(string) + defer C.free(unsafe.Pointer(cString)) + argv[index] = cString + } + requestType := C.uint32_t(Info) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &argv[0]) + resultPayload := <-resultChannel + + return resultPayload.value, resultPayload.errMessage } func (glideRedisClient *GlideRedisClient) CloseClient() { - C.close_client(glideRedisClient.coreClient) + C.close_client(glideRedisClient.coreClient) } diff --git a/go/glide/lib.h b/go/glide/lib.h index ccbb07a4e8..316432ad16 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -136,7 +136,7 @@ void command(const void *client_ptr, * # Safety * Unsafe function because creating string from pointer */ -void log(struct Level log_level, const char *log_identifier, const char *message); +void log_ffi(struct Level log_level, const char *log_identifier, const char *message); /** * # Safety diff --git a/go/src/lib.rs b/go/src/lib.rs index 15dccc7715..411429cdac 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -415,7 +415,7 @@ impl From for logger_core::Level { #[allow(improper_ctypes_definitions)] /// # Safety /// Unsafe function because creating string from pointer -pub unsafe extern "C" fn log( +pub unsafe extern "C" fn log_ffi( log_level: Level, log_identifier: *const c_char, message: *const c_char, From 79343fff985b72c712a7b4987660597f6e9297fb Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 27 Feb 2024 12:17:50 -0800 Subject: [PATCH 03/25] Add license headers --- go/examples/main.go | 3 +++ go/glide/glide.go | 3 +++ go/glide/lib.h | 3 +++ 3 files changed, 9 insertions(+) diff --git a/go/examples/main.go b/go/examples/main.go index 9502f32006..a9ba719ba2 100644 --- a/go/examples/main.go +++ b/go/examples/main.go @@ -1,3 +1,6 @@ +/** + * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 + */ package main import ( diff --git a/go/glide/glide.go b/go/glide/glide.go index 926f9a8551..eb7455f310 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -1,3 +1,6 @@ +/** + * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 + */ package glide /* diff --git a/go/glide/lib.h b/go/glide/lib.h index 316432ad16..98d0955d14 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -1,3 +1,6 @@ +/** + * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 + */ #include #include #include From 2a92415632d39333b758919ce7be814b5e5948ce Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 27 Feb 2024 15:09:42 -0800 Subject: [PATCH 04/25] Refactor cArgs --- go/glide/glide.go | 55 +++++++++++++++++++++-------------------------- go/glide/lib.h | 3 --- 2 files changed, 25 insertions(+), 33 deletions(-) diff --git a/go/glide/glide.go b/go/glide/glide.go index eb7455f310..f8a1bec583 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -139,6 +139,21 @@ func failureCallback(channelPtr C.uintptr_t, errMessage *C.char) { resultChannel <- payload{value: "", errMessage: fmt.Errorf("error at redis operation: %s", goMessage)} } +func stringsToCStrings(args []string) []*C.char { + cArgs := make([]*C.char, len(args)) + for index, string := range args { + cString := C.CString(string) + cArgs[index] = cString + } + return cArgs +} + +func freeCStrings(cArgs []*C.char) { + for _, arg := range cArgs { + C.free(unsafe.Pointer(arg)) + } +} + func (glideRedisClient *GlideRedisClient) ConnectToRedis(host string, port int, useSSL bool, clusterModeEnabled bool) error { caddress := C.CString(host) defer C.free(unsafe.Pointer(caddress)) @@ -162,14 +177,10 @@ func (glideRedisClient *GlideRedisClient) Set(key string, value interface{}) err resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) args := []string{key, strValue} - argv := make([]*C.char, len(args)) - for index, string := range args { - cString := C.CString(string) - defer C.free(unsafe.Pointer(cString)) - argv[index] = cString - } + cArgs := stringsToCStrings(args) + defer freeCStrings(cArgs) requestType := C.uint32_t(SetString) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(2), &argv[0]) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(2), &cArgs[0]) resultPayload := <-resultChannel @@ -184,14 +195,10 @@ func (glideRedisClient *GlideRedisClient) Get(key string) (string, error) { resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) args := []string{key} - argv := make([]*C.char, len(args)) - for index, string := range args { - cString := C.CString(string) - defer C.free(unsafe.Pointer(cString)) - argv[index] = cString - } + cArgs := stringsToCStrings(args) + defer freeCStrings(cArgs) requestType := C.uint32_t(GetString) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(1), &argv[0]) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(1), &cArgs[0]) resultPayload := <-resultChannel return resultPayload.value, nil @@ -201,15 +208,9 @@ func (glideRedisClient *GlideRedisClient) Ping() (string, error) { resultChannel := make(chan payload) resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - args := []string{} - argv := make([]*C.char, len(args)) - for index, string := range args { - cString := C.CString(string) - defer C.free(unsafe.Pointer(cString)) - argv[index] = cString - } + cArgs := []*C.char{} requestType := C.uint32_t(Ping) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &argv[0]) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &cArgs[0]) resultPayload := <-resultChannel return resultPayload.value, resultPayload.errMessage @@ -219,15 +220,9 @@ func (glideRedisClient *GlideRedisClient) Info() (string, error) { resultChannel := make(chan payload) resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - args := []string{} - argv := make([]*C.char, len(args)) - for index, string := range args { - cString := C.CString(string) - defer C.free(unsafe.Pointer(cString)) - argv[index] = cString - } + cArgs := []*C.char{} requestType := C.uint32_t(Info) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &argv[0]) + C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &cArgs[0]) resultPayload := <-resultChannel return resultPayload.value, resultPayload.errMessage diff --git a/go/glide/lib.h b/go/glide/lib.h index 98d0955d14..316432ad16 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -1,6 +1,3 @@ -/** - * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 - */ #include #include #include From 32789424ba568da737899018011a88ddb0dbc8ab Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Thu, 29 Feb 2024 10:15:50 -0800 Subject: [PATCH 05/25] Update API for client creation --- go/examples/main.go | 12 +++++++++++- go/glide/glide.go | 18 ++++++++++++------ go/glide/lib.h | 9 ++++----- go/go.mod | 1 + go/go.sum | 7 +++++++ go/src/lib.rs | 46 ++++++++++++--------------------------------- 6 files changed, 47 insertions(+), 46 deletions(-) diff --git a/go/examples/main.go b/go/examples/main.go index a9ba719ba2..2f88bffd98 100644 --- a/go/examples/main.go +++ b/go/examples/main.go @@ -6,13 +6,23 @@ package main import ( "fmt" "github.com/aws/glide-for-redis/go/glide/glide" + "github.com/aws/glide-for-redis/go/glide/protobuf" ) func main() { fmt.Println("Starting go-glide client...") client := glide.GlideRedisClient{} - err := client.ConnectToRedis("localhost", 6379, false, false) + request := &protobuf.ConnectionRequest{ + TlsMode: protobuf.TlsMode_NoTls, + ClusterModeEnabled: false, + ReadFrom: protobuf.ReadFrom_Primary, + } + request.Addresses = append( + request.Addresses, + &protobuf.NodeAddress{Host: "localhost", Port: uint32(6379)}, + ) + err := client.ConnectToRedis(request) if err != nil { return } diff --git a/go/glide/glide.go b/go/glide/glide.go index f8a1bec583..2b24239457 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -15,6 +15,8 @@ import "C" import ( "fmt" "unsafe" + "github.com/aws/glide-for-redis/go/glide/protobuf" + "github.com/golang/protobuf/proto" ) type GlideRedisClient struct { @@ -154,13 +156,17 @@ func freeCStrings(cArgs []*C.char) { } } -func (glideRedisClient *GlideRedisClient) ConnectToRedis(host string, port int, useSSL bool, clusterModeEnabled bool) error { - caddress := C.CString(host) - defer C.free(unsafe.Pointer(caddress)) - - response := (C.struct_ConnectionResponse)(C.create_client(caddress, C.uint32_t(port), C._Bool(useSSL), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) +func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.ConnectionRequest) error { + marshalledRequest, err := proto.Marshal(request) + if err != nil { + return fmt.Errorf("Failed to encode connection request:", err) + } + byteCount := len(marshalledRequest) + requestBytes := C.CBytes(marshalledRequest) + response := (*C.struct_ConnectionResponse)(C.create_client((*C.uchar)(requestBytes), C.uintptr_t(byteCount), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) + defer C.free(unsafe.Pointer(response)) if response.error != nil { - return fmt.Errorf("Connection error: ", C.GoString(response.error.message)) + return fmt.Errorf(C.GoString(response.error.message)) } glideRedisClient.coreClient = response.conn_ptr return nil diff --git a/go/glide/lib.h b/go/glide/lib.h index 316432ad16..71591c29da 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -118,11 +118,10 @@ typedef void (*FailureCallback)(uintptr_t channel_address, const char *err_messa /** * Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. */ -struct ConnectionResponse create_client(const char *host, - uint32_t port, - bool use_tls, - SuccessCallback success_callback, - FailureCallback failure_callback); +const struct ConnectionResponse *create_client(const uint8_t *connection_request, + uintptr_t request_len, + SuccessCallback success_callback, + FailureCallback failure_callback); void close_client(const void *client_ptr); diff --git a/go/go.mod b/go/go.mod index e5c99d1988..2e4ede8147 100644 --- a/go/go.mod +++ b/go/go.mod @@ -10,6 +10,7 @@ require ( require ( github.com/BurntSushi/toml v1.2.1 // indirect + github.com/golang/protobuf v1.5.3 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/spf13/cobra v1.8.0 // indirect github.com/spf13/pflag v1.0.5 // indirect diff --git a/go/go.sum b/go/go.sum index 4ccef17d5a..26671898ba 100644 --- a/go/go.sum +++ b/go/go.sum @@ -57,11 +57,15 @@ github.com/golang/mock v1.3.1/go.mod h1:sBzyDLLjw3U8JLTeZvSv8jJB+tU5PVekmnlKIyFU github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= +github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= +github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= +github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= github.com/google/pprof v0.0.0-20190515194954-54271f7e092f/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= @@ -272,6 +276,7 @@ golang.org/x/tools v0.12.1-0.20230825192346-2191a27a6dc5 h1:Vk4mysSz+GqQK2eqgWbo golang.org/x/tools v0.12.1-0.20230825192346-2191a27a6dc5/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M= google.golang.org/api v0.8.0/go.mod h1:o4eAsZoiT+ibD93RtjEohWalFOjRDx6CVaqeizhEnKg= @@ -293,6 +298,8 @@ google.golang.org/genproto v0.0.0-20191108220845-16a3f7862a1a/go.mod h1:n3cpQtvx google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= +google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= +google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.32.0 h1:pPC6BG5ex8PDFnkbrGU3EixyhKcQ2aDuBS36lqK/C7I= google.golang.org/protobuf v1.32.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= diff --git a/go/src/lib.rs b/go/src/lib.rs index 411429cdac..e9097f24e7 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -1,8 +1,9 @@ +use glide_core::client::Client as GlideClient; /** * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ use glide_core::connection_request; -use glide_core::{client::Client as GlideClient, connection_request::NodeAddress}; +use protobuf::Message; use redis::{cmd, Cmd, FromRedisValue, RedisResult, Value}; use std::{ ffi::{c_void, CStr, CString}, @@ -52,37 +53,13 @@ pub struct Client { runtime: Runtime, } -fn create_connection_request( - host: String, - port: u32, - use_tls: bool, -) -> connection_request::ConnectionRequest { - let mut address_info = NodeAddress::new(); - address_info.host = host.to_string().into(); - address_info.port = port; - let addresses_info = vec![address_info]; - let mut connection_request = connection_request::ConnectionRequest::new(); - connection_request.addresses = addresses_info; - connection_request.tls_mode = if use_tls { - connection_request::TlsMode::SecureTls - } else { - connection_request::TlsMode::NoTls - } - .into(); - - connection_request -} - fn create_client_internal( - host: *const c_char, - port: u32, - use_tls: bool, + connection_request_bytes: &[u8], success_callback: SuccessCallback, failure_callback: FailureCallback, ) -> RedisResult { - let host_cstring = unsafe { CStr::from_ptr(host as *mut c_char) }; - let host_string = host_cstring.to_str()?.to_string(); - let request = create_connection_request(host_string, port, use_tls); + let request = + connection_request::ConnectionRequest::parse_from_bytes(connection_request_bytes).unwrap(); let runtime = Builder::new_multi_thread() .enable_all() .thread_name("GLIDE for Redis Go thread") @@ -100,13 +77,13 @@ fn create_client_internal( /// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. #[no_mangle] pub extern "C" fn create_client( - host: *const c_char, - port: u32, - use_tls: bool, + connection_request: *const u8, + request_len: usize, success_callback: SuccessCallback, failure_callback: FailureCallback, -) -> ConnectionResponse { - match create_client_internal(host, port, use_tls, success_callback, failure_callback) { +) -> *const ConnectionResponse { + let request_bytes = unsafe { std::slice::from_raw_parts(connection_request, request_len) }; + let response = match create_client_internal(request_bytes, success_callback, failure_callback) { Err(err) => { let message_cstring = CString::new(err.to_string()).unwrap(); ConnectionResponse { @@ -121,7 +98,8 @@ pub extern "C" fn create_client( conn_ptr: Box::into_raw(Box::new(client)) as *const c_void, error: std::ptr::null(), }, - } + }; + Box::into_raw(Box::new(response)) } #[no_mangle] From 2b258e5ff2abc29fa43564a696a55140377a73d3 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Sun, 3 Mar 2024 23:00:40 -0800 Subject: [PATCH 06/25] Remove non-connection logic --- go/.gitignore | 2 +- go/examples/main.go | 12 -- go/glide/glide.go | 162 --------------------- go/src/lib.rs | 335 +------------------------------------------- 4 files changed, 7 insertions(+), 504 deletions(-) diff --git a/go/.gitignore b/go/.gitignore index 12afb3251d..7e45fbb3ff 100644 --- a/go/.gitignore +++ b/go/.gitignore @@ -1,7 +1,7 @@ # Go compilation files *.pb.go -# Examples +# Example binary examples/examples reports diff --git a/go/examples/main.go b/go/examples/main.go index 2f88bffd98..efd9d2267a 100644 --- a/go/examples/main.go +++ b/go/examples/main.go @@ -27,18 +27,6 @@ func main() { return } - err = client.Set("FOO", "BAR") - if err != nil { - panic(err) - } - fmt.Println("SET FOO : BAR") - - val, err := client.Get("FOO") - if err != nil { - panic(err) - } - fmt.Println("GET FOO :", val) - client.CloseClient() fmt.Println("Disconnected from Redis") diff --git a/go/glide/glide.go b/go/glide/glide.go index 2b24239457..7147bdb465 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -30,91 +30,6 @@ type payload struct { type RequestType uint32 -const ( - _ = iota - CustomCommand RequestType = iota - GetString - SetString - Ping - Info - Del - Select - ConfigGet - ConfigSet - ConfigResetStat - ConfigRewrite - ClientGetName - ClientGetRedir - ClientId - ClientInfo - ClientKill - ClientList - ClientNoEvict - ClientNoTouch - ClientPause - ClientReply - ClientSetInfo - ClientSetName - ClientUnblock - ClientUnpause - Expire - HashSet - HashGet - HashDel - HashExists - MGet - MSet - Incr - IncrBy - Decr - IncrByFloat - DecrBy - HashGetAll - HashMSet - HashMGet - HashIncrBy - HashIncrByFloat - LPush - LPop - RPush - RPop - LLen - LRem - LRange - LTrim - SAdd - SRem - SMembers - SCard - PExpireAt - PExpire - ExpireAt - Exists - Unlink - TTL - Zadd - Zrem - Zrange - Zcard - Zcount - ZIncrBy - ZScore - Type - HLen - Echo - ZPopMin - Strlen - Lindex - ZPopMax - XRead - XAdd - XReadGroup - XAck - XTrim - XGroupCreate - XGroupDestroy -) - type ErrorType uint32 const ( @@ -141,21 +56,6 @@ func failureCallback(channelPtr C.uintptr_t, errMessage *C.char) { resultChannel <- payload{value: "", errMessage: fmt.Errorf("error at redis operation: %s", goMessage)} } -func stringsToCStrings(args []string) []*C.char { - cArgs := make([]*C.char, len(args)) - for index, string := range args { - cString := C.CString(string) - cArgs[index] = cString - } - return cArgs -} - -func freeCStrings(cArgs []*C.char) { - for _, arg := range cArgs { - C.free(unsafe.Pointer(arg)) - } -} - func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.ConnectionRequest) error { marshalledRequest, err := proto.Marshal(request) if err != nil { @@ -172,68 +72,6 @@ func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.Conne return nil } -func (glideRedisClient *GlideRedisClient) Set(key string, value interface{}) error { - strValue := fmt.Sprintf("%v", value) - ckey := C.CString(key) - cval := C.CString(strValue) - defer C.free(unsafe.Pointer(ckey)) - defer C.free(unsafe.Pointer(cval)) - - resultChannel := make(chan payload) - resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - - args := []string{key, strValue} - cArgs := stringsToCStrings(args) - defer freeCStrings(cArgs) - requestType := C.uint32_t(SetString) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(2), &cArgs[0]) - - resultPayload := <-resultChannel - - return resultPayload.errMessage -} - -func (glideRedisClient *GlideRedisClient) Get(key string) (string, error) { - ckey := C.CString(key) - defer C.free(unsafe.Pointer(ckey)) - - resultChannel := make(chan payload) - resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - - args := []string{key} - cArgs := stringsToCStrings(args) - defer freeCStrings(cArgs) - requestType := C.uint32_t(GetString) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(1), &cArgs[0]) - resultPayload := <-resultChannel - - return resultPayload.value, nil -} - -func (glideRedisClient *GlideRedisClient) Ping() (string, error) { - resultChannel := make(chan payload) - resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - - cArgs := []*C.char{} - requestType := C.uint32_t(Ping) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &cArgs[0]) - resultPayload := <-resultChannel - - return resultPayload.value, resultPayload.errMessage -} - -func (glideRedisClient *GlideRedisClient) Info() (string, error) { - resultChannel := make(chan payload) - resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) - - cArgs := []*C.char{} - requestType := C.uint32_t(Info) - C.command(glideRedisClient.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(0), &cArgs[0]) - resultPayload := <-resultChannel - - return resultPayload.value, resultPayload.errMessage -} - func (glideRedisClient *GlideRedisClient) CloseClient() { C.close_client(glideRedisClient.coreClient) } diff --git a/go/src/lib.rs b/go/src/lib.rs index e9097f24e7..1436e6da0e 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -4,9 +4,9 @@ use glide_core::client::Client as GlideClient; */ use glide_core::connection_request; use protobuf::Message; -use redis::{cmd, Cmd, FromRedisValue, RedisResult, Value}; +use redis::RedisResult; use std::{ - ffi::{c_void, CStr, CString}, + ffi::{c_void, CString}, os::raw::c_char, }; use tokio::runtime::Builder; @@ -76,13 +76,14 @@ fn create_client_internal( /// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. #[no_mangle] -pub extern "C" fn create_client( +pub unsafe extern "C" fn create_client( connection_request: *const u8, - request_len: usize, + connection_request_len: usize, success_callback: SuccessCallback, failure_callback: FailureCallback, ) -> *const ConnectionResponse { - let request_bytes = unsafe { std::slice::from_raw_parts(connection_request, request_len) }; + let request_bytes = + unsafe { std::slice::from_raw_parts(connection_request, connection_request_len) }; let response = match create_client_internal(request_bytes, success_callback, failure_callback) { Err(err) => { let message_cstring = CString::new(err.to_string()).unwrap(); @@ -108,327 +109,3 @@ pub extern "C" fn close_client(client_ptr: *const c_void) { let _runtime_handle = client_ptr.runtime.enter(); drop(client_ptr); } - -// Cannot use glide_core::redis_request::RequestType, because it is not FFI safe -#[repr(u32)] -pub enum RequestType { - // copied from redis_request.proto - CustomCommand = 1, - GetString = 2, - SetString = 3, - Ping = 4, - Info = 5, - Del = 6, - Select = 7, - ConfigGet = 8, - ConfigSet = 9, - ConfigResetStat = 10, - ConfigRewrite = 11, - ClientGetName = 12, - ClientGetRedir = 13, - ClientId = 14, - ClientInfo = 15, - ClientKill = 16, - ClientList = 17, - ClientNoEvict = 18, - ClientNoTouch = 19, - ClientPause = 20, - ClientReply = 21, - ClientSetInfo = 22, - ClientSetName = 23, - ClientUnblock = 24, - ClientUnpause = 25, - Expire = 26, - HashSet = 27, - HashGet = 28, - HashDel = 29, - HashExists = 30, - MGet = 31, - MSet = 32, - Incr = 33, - IncrBy = 34, - Decr = 35, - IncrByFloat = 36, - DecrBy = 37, - HashGetAll = 38, - HashMSet = 39, - HashMGet = 40, - HashIncrBy = 41, - HashIncrByFloat = 42, - LPush = 43, - LPop = 44, - RPush = 45, - RPop = 46, - LLen = 47, - LRem = 48, - LRange = 49, - LTrim = 50, - SAdd = 51, - SRem = 52, - SMembers = 53, - SCard = 54, - PExpireAt = 55, - PExpire = 56, - ExpireAt = 57, - Exists = 58, - Unlink = 59, - TTL = 60, - Zadd = 61, - Zrem = 62, - Zrange = 63, - Zcard = 64, - Zcount = 65, - ZIncrBy = 66, - ZScore = 67, - Type = 68, - HLen = 69, - Echo = 70, - ZPopMin = 71, - Strlen = 72, - Lindex = 73, - ZPopMax = 74, - XRead = 75, - XAdd = 76, - XReadGroup = 77, - XAck = 78, - XTrim = 79, - XGroupCreate = 80, - XGroupDestroy = 81, -} - -// copied from glide_core::socket_listener::get_command -fn get_command(request_type: RequestType) -> Option { - match request_type { - //RequestType::InvalidRequest => None, - RequestType::CustomCommand => Some(Cmd::new()), - RequestType::GetString => Some(cmd("GET")), - RequestType::SetString => Some(cmd("SET")), - RequestType::Ping => Some(cmd("PING")), - RequestType::Info => Some(cmd("INFO")), - RequestType::Del => Some(cmd("DEL")), - RequestType::Select => Some(cmd("SELECT")), - RequestType::ConfigGet => Some(get_two_word_command("CONFIG", "GET")), - RequestType::ConfigSet => Some(get_two_word_command("CONFIG", "SET")), - RequestType::ConfigResetStat => Some(get_two_word_command("CONFIG", "RESETSTAT")), - RequestType::ConfigRewrite => Some(get_two_word_command("CONFIG", "REWRITE")), - RequestType::ClientGetName => Some(get_two_word_command("CLIENT", "GETNAME")), - RequestType::ClientGetRedir => Some(get_two_word_command("CLIENT", "GETREDIR")), - RequestType::ClientId => Some(get_two_word_command("CLIENT", "ID")), - RequestType::ClientInfo => Some(get_two_word_command("CLIENT", "INFO")), - RequestType::ClientKill => Some(get_two_word_command("CLIENT", "KILL")), - RequestType::ClientList => Some(get_two_word_command("CLIENT", "LIST")), - RequestType::ClientNoEvict => Some(get_two_word_command("CLIENT", "NO-EVICT")), - RequestType::ClientNoTouch => Some(get_two_word_command("CLIENT", "NO-TOUCH")), - RequestType::ClientPause => Some(get_two_word_command("CLIENT", "PAUSE")), - RequestType::ClientReply => Some(get_two_word_command("CLIENT", "REPLY")), - RequestType::ClientSetInfo => Some(get_two_word_command("CLIENT", "SETINFO")), - RequestType::ClientSetName => Some(get_two_word_command("CLIENT", "SETNAME")), - RequestType::ClientUnblock => Some(get_two_word_command("CLIENT", "UNBLOCK")), - RequestType::ClientUnpause => Some(get_two_word_command("CLIENT", "UNPAUSE")), - RequestType::Expire => Some(cmd("EXPIRE")), - RequestType::HashSet => Some(cmd("HSET")), - RequestType::HashGet => Some(cmd("HGET")), - RequestType::HashDel => Some(cmd("HDEL")), - RequestType::HashExists => Some(cmd("HEXISTS")), - RequestType::MSet => Some(cmd("MSET")), - RequestType::MGet => Some(cmd("MGET")), - RequestType::Incr => Some(cmd("INCR")), - RequestType::IncrBy => Some(cmd("INCRBY")), - RequestType::IncrByFloat => Some(cmd("INCRBYFLOAT")), - RequestType::Decr => Some(cmd("DECR")), - RequestType::DecrBy => Some(cmd("DECRBY")), - RequestType::HashGetAll => Some(cmd("HGETALL")), - RequestType::HashMSet => Some(cmd("HMSET")), - RequestType::HashMGet => Some(cmd("HMGET")), - RequestType::HashIncrBy => Some(cmd("HINCRBY")), - RequestType::HashIncrByFloat => Some(cmd("HINCRBYFLOAT")), - RequestType::LPush => Some(cmd("LPUSH")), - RequestType::LPop => Some(cmd("LPOP")), - RequestType::RPush => Some(cmd("RPUSH")), - RequestType::RPop => Some(cmd("RPOP")), - RequestType::LLen => Some(cmd("LLEN")), - RequestType::LRem => Some(cmd("LREM")), - RequestType::LRange => Some(cmd("LRANGE")), - RequestType::LTrim => Some(cmd("LTRIM")), - RequestType::SAdd => Some(cmd("SADD")), - RequestType::SRem => Some(cmd("SREM")), - RequestType::SMembers => Some(cmd("SMEMBERS")), - RequestType::SCard => Some(cmd("SCARD")), - RequestType::PExpireAt => Some(cmd("PEXPIREAT")), - RequestType::PExpire => Some(cmd("PEXPIRE")), - RequestType::ExpireAt => Some(cmd("EXPIREAT")), - RequestType::Exists => Some(cmd("EXISTS")), - RequestType::Unlink => Some(cmd("UNLINK")), - RequestType::TTL => Some(cmd("TTL")), - RequestType::Zadd => Some(cmd("ZADD")), - RequestType::Zrem => Some(cmd("ZREM")), - RequestType::Zrange => Some(cmd("ZRANGE")), - RequestType::Zcard => Some(cmd("ZCARD")), - RequestType::Zcount => Some(cmd("ZCOUNT")), - RequestType::ZIncrBy => Some(cmd("ZINCRBY")), - RequestType::ZScore => Some(cmd("ZSCORE")), - RequestType::Type => Some(cmd("TYPE")), - RequestType::HLen => Some(cmd("HLEN")), - RequestType::Echo => Some(cmd("ECHO")), - RequestType::ZPopMin => Some(cmd("ZPOPMIN")), - RequestType::Strlen => Some(cmd("STRLEN")), - RequestType::Lindex => Some(cmd("LINDEX")), - RequestType::ZPopMax => Some(cmd("ZPOPMAX")), - RequestType::XAck => Some(cmd("XACK")), - RequestType::XAdd => Some(cmd("XADD")), - RequestType::XReadGroup => Some(cmd("XREADGROUP")), - RequestType::XRead => Some(cmd("XREAD")), - RequestType::XGroupCreate => Some(get_two_word_command("XGROUP", "CREATE")), - RequestType::XGroupDestroy => Some(get_two_word_command("XGROUP", "DESTROY")), - RequestType::XTrim => Some(cmd("XTRIM")), - } -} - -// copied from glide_core::socket_listener::get_two_word_command -fn get_two_word_command(first: &str, second: &str) -> Cmd { - let mut cmd = cmd(first); - cmd.arg(second); - cmd -} - -use std::slice::from_raw_parts; -use std::str::Utf8Error; - -pub unsafe fn convert_double_pointer_to_vec( - data: *const *const c_char, - len: usize, -) -> Result, Utf8Error> { - from_raw_parts(data, len) - .iter() - .map(|arg| CStr::from_ptr(*arg).to_str().map(ToString::to_string)) - .collect() -} - -#[no_mangle] -pub unsafe extern "C" fn command( - client_ptr: *const c_void, - channel: usize, - command_type: RequestType, - arg_count: usize, - args: *const *const c_char, -) { - let client = unsafe { Box::leak(Box::from_raw(client_ptr as *mut Client)) }; - // The safety of this needs to be ensured by the calling code. Cannot dispose of the pointer before all operations have completed. - let ptr_address = client_ptr as usize; - - let arg_vec = unsafe { convert_double_pointer_to_vec(args, arg_count) }.unwrap(); // TODO check - - let mut client_clone = client.client.clone(); - client.runtime.spawn(async move { - let mut cmd = get_command(command_type).unwrap(); // TODO check cmd - //print!("{:?}", cmd.args); - cmd.arg(arg_vec); - - let result = client_clone.send_command(&cmd, None).await; - let client = unsafe { Box::leak(Box::from_raw(ptr_address as *mut Client)) }; - let value = match result { - Ok(value) => value, - Err(err) => { - print!(" === err {:?}\n", err); - let c_err_str = CString::new(err.to_string()).expect("CString::new failed"); - unsafe { (client.failure_callback)(channel, c_err_str.as_ptr()) }; - return; - } - }; - - //print!(" === val {:?}\n", value.clone()); - - let result: RedisResult> = match value { - Value::Nil => Ok(None), - Value::Int(num) => Ok(Some(CString::new(format!("{}", num)).unwrap())), - Value::SimpleString(_) | Value::BulkString(_) => { - Option::::from_owned_redis_value(value) - } - Value::Okay => Ok(Some(CString::new("OK").unwrap())), - Value::Double(num) => Ok(Some(CString::new(format!("{}", num)).unwrap())), - Value::Boolean(bool) => Ok(Some(CString::new(format!("{}", bool)).unwrap())), - _ => todo!(), - }; - - //print!(" === result2 {:?}\n", result); - - unsafe { - match result { - Ok(None) => (client.success_callback)(channel, std::ptr::null()), - Ok(Some(c_str)) => (client.success_callback)(channel, c_str.as_ptr()), - Err(err) => { - let c_err_str = CString::new(err.to_string()).expect("CString::new failed"); - (client.failure_callback)(channel, c_err_str.as_ptr()); - } - }; - } - }); -} - -impl From for Level { - fn from(level: logger_core::Level) -> Self { - match level { - logger_core::Level::Error => Level::Error, - logger_core::Level::Warn => Level::Warn, - logger_core::Level::Info => Level::Info, - logger_core::Level::Debug => Level::Debug, - logger_core::Level::Trace => Level::Trace, - } - } -} - -impl From for logger_core::Level { - fn from(level: Level) -> logger_core::Level { - match level { - Level::Error => logger_core::Level::Error, - Level::Warn => logger_core::Level::Warn, - Level::Info => logger_core::Level::Info, - Level::Debug => logger_core::Level::Debug, - Level::Trace => logger_core::Level::Trace, - } - } -} - -#[no_mangle] -#[allow(improper_ctypes_definitions)] -/// # Safety -/// Unsafe function because creating string from pointer -pub unsafe extern "C" fn log_ffi( - log_level: Level, - log_identifier: *const c_char, - message: *const c_char, -) { - unsafe { - logger_core::log( - log_level.into(), - CStr::from_ptr(log_identifier) - .to_str() - .expect("Can not read log_identifier argument."), - CStr::from_ptr(message) - .to_str() - .expect("Can not read message argument."), - ); - } -} - -#[no_mangle] -#[allow(improper_ctypes_definitions)] -/// # Safety -/// Unsafe function because creating string from pointer -pub unsafe extern "C" fn init(level: Option, file_name: *const c_char) -> Level { - let file_name_as_str; - unsafe { - file_name_as_str = if file_name.is_null() { - None - } else { - Some( - CStr::from_ptr(file_name) - .to_str() - .expect("Can not read string argument."), - ) - }; - - let logger_level = logger_core::init(level.map(|level| level.into()), file_name_as_str); - logger_level.into() - } -} From 6ccc6c1401ea3bbff0ac2de11d6817b77941b267 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Sun, 3 Mar 2024 23:01:40 -0800 Subject: [PATCH 07/25] Update lib.h --- go/glide/lib.h | 109 +------------------------------------------------ 1 file changed, 1 insertion(+), 108 deletions(-) diff --git a/go/glide/lib.h b/go/glide/lib.h index 71591c29da..c90199f86a 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -12,95 +12,6 @@ enum ErrorType { }; typedef uint32_t ErrorType; -enum RequestType { - CustomCommand = 1, - GetString = 2, - SetString = 3, - Ping = 4, - Info = 5, - Del = 6, - Select = 7, - ConfigGet = 8, - ConfigSet = 9, - ConfigResetStat = 10, - ConfigRewrite = 11, - ClientGetName = 12, - ClientGetRedir = 13, - ClientId = 14, - ClientInfo = 15, - ClientKill = 16, - ClientList = 17, - ClientNoEvict = 18, - ClientNoTouch = 19, - ClientPause = 20, - ClientReply = 21, - ClientSetInfo = 22, - ClientSetName = 23, - ClientUnblock = 24, - ClientUnpause = 25, - Expire = 26, - HashSet = 27, - HashGet = 28, - HashDel = 29, - HashExists = 30, - MGet = 31, - MSet = 32, - Incr = 33, - IncrBy = 34, - Decr = 35, - IncrByFloat = 36, - DecrBy = 37, - HashGetAll = 38, - HashMSet = 39, - HashMGet = 40, - HashIncrBy = 41, - HashIncrByFloat = 42, - LPush = 43, - LPop = 44, - RPush = 45, - RPop = 46, - LLen = 47, - LRem = 48, - LRange = 49, - LTrim = 50, - SAdd = 51, - SRem = 52, - SMembers = 53, - SCard = 54, - PExpireAt = 55, - PExpire = 56, - ExpireAt = 57, - Exists = 58, - Unlink = 59, - TTL = 60, - Zadd = 61, - Zrem = 62, - Zrange = 63, - Zcard = 64, - Zcount = 65, - ZIncrBy = 66, - ZScore = 67, - Type = 68, - HLen = 69, - Echo = 70, - ZPopMin = 71, - Strlen = 72, - Lindex = 73, - ZPopMax = 74, - XRead = 75, - XAdd = 76, - XReadGroup = 77, - XAck = 78, - XTrim = 79, - XGroupCreate = 80, - XGroupDestroy = 81, -}; -typedef uint32_t RequestType; - -typedef struct Level Level; - -typedef struct Option_Level Option_Level; - typedef struct RedisErrorFFI { const char *message; ErrorType error_type; @@ -119,26 +30,8 @@ typedef void (*FailureCallback)(uintptr_t channel_address, const char *err_messa * Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. */ const struct ConnectionResponse *create_client(const uint8_t *connection_request, - uintptr_t request_len, + uintptr_t connection_request_len, SuccessCallback success_callback, FailureCallback failure_callback); void close_client(const void *client_ptr); - -void command(const void *client_ptr, - uintptr_t channel, - RequestType command_type, - uintptr_t arg_count, - const char *const *args); - -/** - * # Safety - * Unsafe function because creating string from pointer - */ -void log_ffi(struct Level log_level, const char *log_identifier, const char *message); - -/** - * # Safety - * Unsafe function because creating string from pointer - */ -struct Level init(struct Option_Level level, const char *file_name); From bdf9c19e8f5b951a20b30e6c38ed5364c0d5c9f0 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 5 Mar 2024 14:27:31 -0800 Subject: [PATCH 08/25] Add additional documentation to lib.rs functions --- go/examples/main.go | 1 + go/glide/lib.h | 26 ++++++++++++++++++++++++-- go/src/lib.rs | 35 ++++++++++++++++++++++------------- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/go/examples/main.go b/go/examples/main.go index efd9d2267a..aa4d69c8ca 100644 --- a/go/examples/main.go +++ b/go/examples/main.go @@ -13,6 +13,7 @@ func main() { fmt.Println("Starting go-glide client...") client := glide.GlideRedisClient{} + // TODO: Update when configuration is implemented request := &protobuf.ConnectionRequest{ TlsMode: protobuf.TlsMode_NoTls, ClusterModeEnabled: false, diff --git a/go/glide/lib.h b/go/glide/lib.h index c90199f86a..d176452769 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -22,16 +22,38 @@ typedef struct ConnectionResponse { const struct RedisErrorFFI *error; } ConnectionResponse; +/** + * Success callback that is called when a Redis command succeeds. + */ typedef void (*SuccessCallback)(uintptr_t channel_address, const char *message); -typedef void (*FailureCallback)(uintptr_t channel_address, const char *err_message); +/** + * Failure callback that is called when a Redis command fails. + */ +typedef void (*FailureCallback)(uintptr_t channel_address, const struct RedisErrorFFI *error); /** * Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. + * + * # Panics + * `create_client` will panic if the given `connection_request_bytes` fail to parse into a protobuf `ConnectionRequest`. + * + * # Safety + * + * * `connection_request_bytes` must point to `connection_request_len` consecutive properly initialized bytes. + * * `connection_request_len` must not be greater than `isize::MAX`. See the safety documentation of [`std::slice::from_raw_parts`](https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html). */ -const struct ConnectionResponse *create_client(const uint8_t *connection_request, +const struct ConnectionResponse *create_client(const uint8_t *connection_request_bytes, uintptr_t connection_request_len, SuccessCallback success_callback, FailureCallback failure_callback); +/** + * Closes the given client, deallocating it from the heap. + * + * # Safety + * + * * `client_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). + * * `client_ptr` must not be null. + */ void close_client(const void *client_ptr); diff --git a/go/src/lib.rs b/go/src/lib.rs index 1436e6da0e..2652fd7e4d 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -1,7 +1,7 @@ -use glide_core::client::Client as GlideClient; /** * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 */ +use glide_core::client::Client as GlideClient; use glide_core::connection_request; use protobuf::Message; use redis::RedisResult; @@ -12,18 +12,13 @@ use std::{ use tokio::runtime::Builder; use tokio::runtime::Runtime; +/// Success callback that is called when a Redis command succeeds. pub type SuccessCallback = unsafe extern "C" fn(channel_address: usize, message: *const c_char) -> (); -pub type FailureCallback = - unsafe extern "C" fn(channel_address: usize, err_message: *const c_char) -> (); -pub enum Level { - Error = 0, - Warn = 1, - Info = 2, - Debug = 3, - Trace = 4, -} +/// Failure callback that is called when a Redis command fails. +pub type FailureCallback = + unsafe extern "C" fn(channel_address: usize, error: *const RedisErrorFFI) -> (); #[repr(C)] pub struct ConnectionResponse { @@ -75,15 +70,23 @@ fn create_client_internal( } /// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. +/// +/// # Panics +/// `create_client` will panic if the given `connection_request_bytes` fail to parse into a protobuf `ConnectionRequest`. +/// +/// # Safety +/// +/// * `connection_request_bytes` must point to `connection_request_len` consecutive properly initialized bytes. +/// * `connection_request_len` must not be greater than `isize::MAX`. See the safety documentation of [`std::slice::from_raw_parts`](https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html). #[no_mangle] pub unsafe extern "C" fn create_client( - connection_request: *const u8, + connection_request_bytes: *const u8, connection_request_len: usize, success_callback: SuccessCallback, failure_callback: FailureCallback, ) -> *const ConnectionResponse { let request_bytes = - unsafe { std::slice::from_raw_parts(connection_request, connection_request_len) }; + unsafe { std::slice::from_raw_parts(connection_request_bytes, connection_request_len) }; let response = match create_client_internal(request_bytes, success_callback, failure_callback) { Err(err) => { let message_cstring = CString::new(err.to_string()).unwrap(); @@ -103,8 +106,14 @@ pub unsafe extern "C" fn create_client( Box::into_raw(Box::new(response)) } +/// Closes the given client, deallocating it from the heap. +/// +/// # Safety +/// +/// * `client_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). +/// * `client_ptr` must not be null. #[no_mangle] -pub extern "C" fn close_client(client_ptr: *const c_void) { +pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { let client_ptr = unsafe { Box::from_raw(client_ptr as *mut Client) }; let _runtime_handle = client_ptr.runtime.enter(); drop(client_ptr); From 4bd25cc33a7b13950ff0b6ee651d34c2df458f5c Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 5 Mar 2024 17:36:57 -0800 Subject: [PATCH 09/25] Properly free connection response and error --- go/glide/glide.go | 9 ++--- go/glide/lib.h | 43 +++++++++++++++++++---- go/src/lib.rs | 89 ++++++++++++++++++++++++++++++++++++----------- 3 files changed, 109 insertions(+), 32 deletions(-) diff --git a/go/glide/glide.go b/go/glide/glide.go index 7147bdb465..9e46402f95 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -8,7 +8,7 @@ package glide #include "lib.h" void successCallback(uintptr_t channelPtr, char *message); -void failureCallback(uintptr_t channelPtr, char *errMessage); +void failureCallback(uintptr_t channelPtr, struct RedisErrorFFI *errMessage); */ import "C" @@ -49,8 +49,8 @@ func successCallback(channelPtr C.uintptr_t, message *C.char) { } //export failureCallback -func failureCallback(channelPtr C.uintptr_t, errMessage *C.char) { - goMessage := C.GoString(errMessage) +func failureCallback(channelPtr C.uintptr_t, errMessage *C.RedisErrorFFI) { + goMessage := C.GoString(errMessage.message) goChannelPointer := uintptr(channelPtr) resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) resultChannel <- payload{value: "", errMessage: fmt.Errorf("error at redis operation: %s", goMessage)} @@ -64,8 +64,9 @@ func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.Conne byteCount := len(marshalledRequest) requestBytes := C.CBytes(marshalledRequest) response := (*C.struct_ConnectionResponse)(C.create_client((*C.uchar)(requestBytes), C.uintptr_t(byteCount), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) - defer C.free(unsafe.Pointer(response)) + defer C.free_connection_response(response) if response.error != nil { + defer C.free_error(response.error) return fmt.Errorf(C.GoString(response.error.message)) } glideRedisClient.coreClient = response.conn_ptr diff --git a/go/glide/lib.h b/go/glide/lib.h index d176452769..096937e167 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -3,20 +3,30 @@ #include #include -enum ErrorType { +/** + * FFI compatible version of the ErrorType enum defined in protobuf. + */ +typedef enum ErrorType { ClosingError = 0, RequestError = 1, TimeoutError = 2, ExecAbortError = 3, ConnectionError = 4, -}; -typedef uint32_t ErrorType; +} ErrorType; +/** + * A Redis error. + */ typedef struct RedisErrorFFI { const char *message; - ErrorType error_type; + enum ErrorType error_type; } RedisErrorFFI; +/** + * The connection response. + * + * It contains either a connection or an error. It is represented as a struct instead of an enum for ease of use in the wrapper language. + */ typedef struct ConnectionResponse { const void *conn_ptr; const struct RedisErrorFFI *error; @@ -35,9 +45,6 @@ typedef void (*FailureCallback)(uintptr_t channel_address, const struct RedisErr /** * Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. * - * # Panics - * `create_client` will panic if the given `connection_request_bytes` fail to parse into a protobuf `ConnectionRequest`. - * * # Safety * * * `connection_request_bytes` must point to `connection_request_len` consecutive properly initialized bytes. @@ -57,3 +64,25 @@ const struct ConnectionResponse *create_client(const uint8_t *connection_request * * `client_ptr` must not be null. */ void close_client(const void *client_ptr); + +/** + * Deallocates a `ConnectionResponse`. + * + * # Safety + * + * * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). + * * `connection_response_ptr` must not be null. + */ +void free_connection_response(const struct ConnectionResponse *connection_response_ptr); + +/** + * Deallocates a `RedisErrorFFI`. + * + * # Safety + * + * * `error_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). + * * The error message must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). + * * `error_ptr` must not be null. + * * The error message pointer must not be null. + */ +void free_error(const struct RedisErrorFFI *error_ptr); diff --git a/go/src/lib.rs b/go/src/lib.rs index 2652fd7e4d..955f0b4ab9 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -4,7 +4,6 @@ use glide_core::client::Client as GlideClient; use glide_core::connection_request; use protobuf::Message; -use redis::RedisResult; use std::{ ffi::{c_void, CString}, os::raw::c_char, @@ -20,19 +19,25 @@ pub type SuccessCallback = pub type FailureCallback = unsafe extern "C" fn(channel_address: usize, error: *const RedisErrorFFI) -> (); +/// The connection response. +/// +/// It contains either a connection or an error. It is represented as a struct instead of an enum for ease of use in the wrapper language. #[repr(C)] pub struct ConnectionResponse { conn_ptr: *const c_void, error: *const RedisErrorFFI, } +/// A Redis error. #[repr(C)] pub struct RedisErrorFFI { message: *const c_char, error_type: ErrorType, } -#[repr(u32)] +/// FFI compatible version of the ErrorType enum defined in protobuf. +// TODO: Update when command errors are implemented +#[repr(C)] pub enum ErrorType { ClosingError = 0, RequestError = 1, @@ -41,10 +46,11 @@ pub enum ErrorType { ConnectionError = 4, } +/// The glide client. pub struct Client { client: GlideClient, success_callback: SuccessCallback, - failure_callback: FailureCallback, // TODO - add specific error codes + failure_callback: FailureCallback, runtime: Runtime, } @@ -52,15 +58,34 @@ fn create_client_internal( connection_request_bytes: &[u8], success_callback: SuccessCallback, failure_callback: FailureCallback, -) -> RedisResult { - let request = - connection_request::ConnectionRequest::parse_from_bytes(connection_request_bytes).unwrap(); +) -> Result { + let request = connection_request::ConnectionRequest::parse_from_bytes(connection_request_bytes) + .map_err(|err| { + let msg = CString::new(err.to_string()).unwrap(); + RedisErrorFFI { + message: msg.into_raw(), + error_type: ErrorType::ConnectionError, + } + })?; let runtime = Builder::new_multi_thread() .enable_all() .thread_name("GLIDE for Redis Go thread") - .build()?; + .build() + .map_err(|err| { + let msg = CString::new(err.to_string()).unwrap(); + RedisErrorFFI { + message: msg.into_raw(), + error_type: ErrorType::ConnectionError, + } + })?; let _runtime_handle = runtime.enter(); - let client = runtime.block_on(GlideClient::new(request)).unwrap(); // TODO - handle errors. + let client = runtime.block_on(GlideClient::new(request)).map_err(|err| { + let msg = CString::new(err.to_string()).unwrap(); + RedisErrorFFI { + message: msg.into_raw(), + error_type: ErrorType::ConnectionError, + } + })?; Ok(Client { client, success_callback, @@ -71,9 +96,6 @@ fn create_client_internal( /// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. /// -/// # Panics -/// `create_client` will panic if the given `connection_request_bytes` fail to parse into a protobuf `ConnectionRequest`. -/// /// # Safety /// /// * `connection_request_bytes` must point to `connection_request_len` consecutive properly initialized bytes. @@ -88,16 +110,10 @@ pub unsafe extern "C" fn create_client( let request_bytes = unsafe { std::slice::from_raw_parts(connection_request_bytes, connection_request_len) }; let response = match create_client_internal(request_bytes, success_callback, failure_callback) { - Err(err) => { - let message_cstring = CString::new(err.to_string()).unwrap(); - ConnectionResponse { - conn_ptr: std::ptr::null(), - error: &RedisErrorFFI { - message: message_cstring.as_ptr(), - error_type: ErrorType::ConnectionError, - }, - } - } + Err(err) => ConnectionResponse { + conn_ptr: std::ptr::null(), + error: Box::into_raw(Box::new(err)) as *const RedisErrorFFI, + }, Ok(client) => ConnectionResponse { conn_ptr: Box::into_raw(Box::new(client)) as *const c_void, error: std::ptr::null(), @@ -118,3 +134,34 @@ pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { let _runtime_handle = client_ptr.runtime.enter(); drop(client_ptr); } + +/// Deallocates a `ConnectionResponse`. +/// +/// # Safety +/// +/// * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). +/// * `connection_response_ptr` must not be null. +#[no_mangle] +pub unsafe extern "C" fn free_connection_response( + connection_response_ptr: *const ConnectionResponse, +) { + let connection_response = + unsafe { Box::from_raw(connection_response_ptr as *mut ConnectionResponse) }; + drop(connection_response); +} + +/// Deallocates a `RedisErrorFFI`. +/// +/// # Safety +/// +/// * `error_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). +/// * The error message must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `error_ptr` must not be null. +/// * The error message pointer must not be null. +#[no_mangle] +pub unsafe extern "C" fn free_error(error_ptr: *const RedisErrorFFI) { + let error = unsafe { Box::from_raw(error_ptr as *mut RedisErrorFFI) }; + let error_msg = unsafe { CString::from_raw(error.message as *mut c_char) }; + drop(error); + drop(error_msg); +} From dc4df76f15aa6341ec3c7f8cf518e0530b63b1ec Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 5 Mar 2024 17:39:00 -0800 Subject: [PATCH 10/25] Run go fmt --- go/examples/main.go | 2 +- go/glide/glide.go | 70 ++++++++++++++++++++++----------------------- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/go/examples/main.go b/go/examples/main.go index aa4d69c8ca..2fd30c16e3 100644 --- a/go/examples/main.go +++ b/go/examples/main.go @@ -19,7 +19,7 @@ func main() { ClusterModeEnabled: false, ReadFrom: protobuf.ReadFrom_Primary, } - request.Addresses = append( + request.Addresses = append( request.Addresses, &protobuf.NodeAddress{Host: "localhost", Port: uint32(6379)}, ) diff --git a/go/glide/glide.go b/go/glide/glide.go index 9e46402f95..8648df736e 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -13,19 +13,19 @@ void failureCallback(uintptr_t channelPtr, struct RedisErrorFFI *errMessage); import "C" import ( - "fmt" - "unsafe" - "github.com/aws/glide-for-redis/go/glide/protobuf" - "github.com/golang/protobuf/proto" + "fmt" + "github.com/aws/glide-for-redis/go/glide/protobuf" + "github.com/golang/protobuf/proto" + "unsafe" ) type GlideRedisClient struct { - coreClient unsafe.Pointer + coreClient unsafe.Pointer } type payload struct { - value string - errMessage error + value string + errMessage error } type RequestType uint32 @@ -33,46 +33,46 @@ type RequestType uint32 type ErrorType uint32 const ( - ClosingError = iota - RequestError - TimeoutError - ExecAbortError - ConnectionError + ClosingError = iota + RequestError + TimeoutError + ExecAbortError + ConnectionError ) //export successCallback func successCallback(channelPtr C.uintptr_t, message *C.char) { - goMessage := C.GoString(message) - goChannelPointer := uintptr(channelPtr) - resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) - resultChannel <- payload{value: goMessage, errMessage: nil} + goMessage := C.GoString(message) + goChannelPointer := uintptr(channelPtr) + resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) + resultChannel <- payload{value: goMessage, errMessage: nil} } //export failureCallback func failureCallback(channelPtr C.uintptr_t, errMessage *C.RedisErrorFFI) { - goMessage := C.GoString(errMessage.message) - goChannelPointer := uintptr(channelPtr) - resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) - resultChannel <- payload{value: "", errMessage: fmt.Errorf("error at redis operation: %s", goMessage)} + goMessage := C.GoString(errMessage.message) + goChannelPointer := uintptr(channelPtr) + resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) + resultChannel <- payload{value: "", errMessage: fmt.Errorf("error at redis operation: %s", goMessage)} } func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.ConnectionRequest) error { - marshalledRequest, err := proto.Marshal(request) - if err != nil { - return fmt.Errorf("Failed to encode connection request:", err) - } - byteCount := len(marshalledRequest) - requestBytes := C.CBytes(marshalledRequest) - response := (*C.struct_ConnectionResponse)(C.create_client((*C.uchar)(requestBytes), C.uintptr_t(byteCount), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) - defer C.free_connection_response(response) - if response.error != nil { - defer C.free_error(response.error) - return fmt.Errorf(C.GoString(response.error.message)) - } - glideRedisClient.coreClient = response.conn_ptr - return nil + marshalledRequest, err := proto.Marshal(request) + if err != nil { + return fmt.Errorf("Failed to encode connection request:", err) + } + byteCount := len(marshalledRequest) + requestBytes := C.CBytes(marshalledRequest) + response := (*C.struct_ConnectionResponse)(C.create_client((*C.uchar)(requestBytes), C.uintptr_t(byteCount), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) + defer C.free_connection_response(response) + if response.error != nil { + defer C.free_error(response.error) + return fmt.Errorf(C.GoString(response.error.message)) + } + glideRedisClient.coreClient = response.conn_ptr + return nil } func (glideRedisClient *GlideRedisClient) CloseClient() { - C.close_client(glideRedisClient.coreClient) + C.close_client(glideRedisClient.coreClient) } From cd5c2dad95b8700affdb6010c4d48161101f29e9 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 5 Mar 2024 18:02:48 -0800 Subject: [PATCH 11/25] Add error for when user closes client before creating it --- go/examples/main.go | 12 ++++++++---- go/glide/glide.go | 6 +++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/go/examples/main.go b/go/examples/main.go index 2fd30c16e3..1c0dc9ffef 100644 --- a/go/examples/main.go +++ b/go/examples/main.go @@ -7,6 +7,7 @@ import ( "fmt" "github.com/aws/glide-for-redis/go/glide/glide" "github.com/aws/glide-for-redis/go/glide/protobuf" + "log" ) func main() { @@ -23,12 +24,15 @@ func main() { request.Addresses, &protobuf.NodeAddress{Host: "localhost", Port: uint32(6379)}, ) - err := client.ConnectToRedis(request) - if err != nil { - return + connectionErr := client.ConnectToRedis(request) + if connectionErr != nil { + log.Fatal(connectionErr) } - client.CloseClient() + closeClientErr := client.CloseClient() + if closeClientErr != nil { + log.Fatal(closeClientErr) + } fmt.Println("Disconnected from Redis") } diff --git a/go/glide/glide.go b/go/glide/glide.go index 8648df736e..4d7197e152 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -73,6 +73,10 @@ func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.Conne return nil } -func (glideRedisClient *GlideRedisClient) CloseClient() { +func (glideRedisClient *GlideRedisClient) CloseClient() error { + if glideRedisClient.coreClient == nil { + return fmt.Errorf("Cannot close glide client before it has been created.") + } C.close_client(glideRedisClient.coreClient) + return nil } From 055e74bc9db58be59ea27675461272c82c3029d8 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 5 Mar 2024 18:09:36 -0800 Subject: [PATCH 12/25] Add doc comments about avoiding memory leaks --- go/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/go/src/lib.rs b/go/src/lib.rs index 955f0b4ab9..e765d85bf5 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -16,6 +16,8 @@ pub type SuccessCallback = unsafe extern "C" fn(channel_address: usize, message: *const c_char) -> (); /// Failure callback that is called when a Redis command fails. +/// +/// `error` should be manually freed by calling `free_error` after this callback is invoked, otherwise a memory leak will occur. pub type FailureCallback = unsafe extern "C" fn(channel_address: usize, error: *const RedisErrorFFI) -> (); @@ -96,6 +98,8 @@ fn create_client_internal( /// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. /// +/// The returned `ConnectionResponse` should be manually freed by calling `free_connection_response`, otherwise a memory leak will occur. It should be freed whether or not an error occurs. +/// /// # Safety /// /// * `connection_request_bytes` must point to `connection_request_len` consecutive properly initialized bytes. From f23b01a379bd684db07f1cafe955d619282e3085 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 5 Mar 2024 18:16:23 -0800 Subject: [PATCH 13/25] Allow unused Client fields for now --- go/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/go/src/lib.rs b/go/src/lib.rs index e765d85bf5..71a034885c 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -49,6 +49,7 @@ pub enum ErrorType { } /// The glide client. +#[allow(dead_code)] pub struct Client { client: GlideClient, success_callback: SuccessCallback, From aa1fb089889858164fcb0bfd793414a8b56993fd Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Tue, 5 Mar 2024 19:58:04 -0800 Subject: [PATCH 14/25] Add formatting directive to fmt.Errorf --- go/glide/glide.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/glide/glide.go b/go/glide/glide.go index 4d7197e152..5cebe6b363 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -59,7 +59,7 @@ func failureCallback(channelPtr C.uintptr_t, errMessage *C.RedisErrorFFI) { func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.ConnectionRequest) error { marshalledRequest, err := proto.Marshal(request) if err != nil { - return fmt.Errorf("Failed to encode connection request:", err) + return fmt.Errorf("Failed to encode connection request: %v", err) } byteCount := len(marshalledRequest) requestBytes := C.CBytes(marshalledRequest) From 2fe5bdda00673cea5c70ec109743799edb4fcf36 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Wed, 6 Mar 2024 14:16:42 -0800 Subject: [PATCH 15/25] Replace callback implementations with TODO implementations --- go/glide/glide.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/go/glide/glide.go b/go/glide/glide.go index 5cebe6b363..18604883fa 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -42,18 +42,12 @@ const ( //export successCallback func successCallback(channelPtr C.uintptr_t, message *C.char) { - goMessage := C.GoString(message) - goChannelPointer := uintptr(channelPtr) - resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) - resultChannel <- payload{value: goMessage, errMessage: nil} + // TODO: Implement when we implement the command logic } //export failureCallback func failureCallback(channelPtr C.uintptr_t, errMessage *C.RedisErrorFFI) { - goMessage := C.GoString(errMessage.message) - goChannelPointer := uintptr(channelPtr) - resultChannel := *(*chan payload)(unsafe.Pointer(goChannelPointer)) - resultChannel <- payload{value: "", errMessage: fmt.Errorf("error at redis operation: %s", goMessage)} + // TODO: Implement when we implement the command logic } func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.ConnectionRequest) error { From 53c8d7d93def5196671b1fa59ff85cdf306e859c Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Wed, 6 Mar 2024 17:45:36 -0800 Subject: [PATCH 16/25] Remove RedisErrorFFI struct and use glide-core RequetErrorType instead --- go/glide/glide.go | 10 +++--- go/glide/lib.h | 49 +++++++++++++------------- go/src/lib.rs | 88 ++++++++++++++++++++--------------------------- 3 files changed, 66 insertions(+), 81 deletions(-) diff --git a/go/glide/glide.go b/go/glide/glide.go index 18604883fa..ae16cb0ecf 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -8,7 +8,7 @@ package glide #include "lib.h" void successCallback(uintptr_t channelPtr, char *message); -void failureCallback(uintptr_t channelPtr, struct RedisErrorFFI *errMessage); +void failureCallback(uintptr_t channelPtr, char *errMessage, RequestErrorType errType); */ import "C" @@ -46,7 +46,7 @@ func successCallback(channelPtr C.uintptr_t, message *C.char) { } //export failureCallback -func failureCallback(channelPtr C.uintptr_t, errMessage *C.RedisErrorFFI) { +func failureCallback(channelPtr C.uintptr_t, errMessage *C.char, errType C.RequestErrorType) { // TODO: Implement when we implement the command logic } @@ -59,9 +59,9 @@ func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.Conne requestBytes := C.CBytes(marshalledRequest) response := (*C.struct_ConnectionResponse)(C.create_client((*C.uchar)(requestBytes), C.uintptr_t(byteCount), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) defer C.free_connection_response(response) - if response.error != nil { - defer C.free_error(response.error) - return fmt.Errorf(C.GoString(response.error.message)) + if response.error_message != nil { + defer C.free_error(response.error_message) + return fmt.Errorf(C.GoString(response.error_message)) } glideRedisClient.coreClient = response.conn_ptr return nil diff --git a/go/glide/lib.h b/go/glide/lib.h index 096937e167..3317c47a9c 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -1,26 +1,18 @@ +/* + * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 + */ + #include #include #include #include -/** - * FFI compatible version of the ErrorType enum defined in protobuf. - */ -typedef enum ErrorType { - ClosingError = 0, - RequestError = 1, - TimeoutError = 2, - ExecAbortError = 3, - ConnectionError = 4, -} ErrorType; - -/** - * A Redis error. - */ -typedef struct RedisErrorFFI { - const char *message; - enum ErrorType error_type; -} RedisErrorFFI; +typedef enum RequestErrorType { + Unspecified = 0, + ExecAbort = 1, + Timeout = 2, + Disconnect = 3, +} RequestErrorType; /** * The connection response. @@ -29,7 +21,8 @@ typedef struct RedisErrorFFI { */ typedef struct ConnectionResponse { const void *conn_ptr; - const struct RedisErrorFFI *error; + const char *error_message; + RequestErrorType error_type; } ConnectionResponse; /** @@ -39,12 +32,18 @@ typedef void (*SuccessCallback)(uintptr_t channel_address, const char *message); /** * Failure callback that is called when a Redis command fails. + * + * `error` should be manually freed by calling `free_error` after this callback is invoked, otherwise a memory leak will occur. */ -typedef void (*FailureCallback)(uintptr_t channel_address, const struct RedisErrorFFI *error); +typedef void (*FailureCallback)(uintptr_t channel_address, + const char *error_message, + RequestErrorType error_type); /** * Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. * + * The returned `ConnectionResponse` should be manually freed by calling `free_connection_response`, otherwise a memory leak will occur. It should be freed whether or not an error occurs. + * * # Safety * * * `connection_request_bytes` must point to `connection_request_len` consecutive properly initialized bytes. @@ -76,13 +75,11 @@ void close_client(const void *client_ptr); void free_connection_response(const struct ConnectionResponse *connection_response_ptr); /** - * Deallocates a `RedisErrorFFI`. + * Deallocates an error message `CString`. * * # Safety * - * * `error_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). - * * The error message must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). - * * `error_ptr` must not be null. - * * The error message pointer must not be null. + * * `error_msg_ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). + * * `error_msg_ptr` must not be null. */ -void free_error(const struct RedisErrorFFI *error_ptr); +void free_error(const char *error_msg_ptr); diff --git a/go/src/lib.rs b/go/src/lib.rs index 71a034885c..76194eb96e 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -3,6 +3,8 @@ */ use glide_core::client::Client as GlideClient; use glide_core::connection_request; +use glide_core::errors; +use glide_core::errors::RequestErrorType; use protobuf::Message; use std::{ ffi::{c_void, CString}, @@ -12,14 +14,18 @@ use tokio::runtime::Builder; use tokio::runtime::Runtime; /// Success callback that is called when a Redis command succeeds. +// TODO: Change message type when implementing command logic pub type SuccessCallback = unsafe extern "C" fn(channel_address: usize, message: *const c_char) -> (); /// Failure callback that is called when a Redis command fails. /// /// `error` should be manually freed by calling `free_error` after this callback is invoked, otherwise a memory leak will occur. -pub type FailureCallback = - unsafe extern "C" fn(channel_address: usize, error: *const RedisErrorFFI) -> (); +pub type FailureCallback = unsafe extern "C" fn( + channel_address: usize, + error_message: *const c_char, + error_type: RequestErrorType, +) -> (); /// The connection response. /// @@ -27,25 +33,8 @@ pub type FailureCallback = #[repr(C)] pub struct ConnectionResponse { conn_ptr: *const c_void, - error: *const RedisErrorFFI, -} - -/// A Redis error. -#[repr(C)] -pub struct RedisErrorFFI { - message: *const c_char, - error_type: ErrorType, -} - -/// FFI compatible version of the ErrorType enum defined in protobuf. -// TODO: Update when command errors are implemented -#[repr(C)] -pub enum ErrorType { - ClosingError = 0, - RequestError = 1, - TimeoutError = 2, - ExecAbortError = 3, - ConnectionError = 4, + error_message: *const c_char, + error_type: RequestErrorType, } /// The glide client. @@ -57,38 +46,39 @@ pub struct Client { runtime: Runtime, } +struct CreateClientError { + message: String, + error_type: RequestErrorType, +} + fn create_client_internal( connection_request_bytes: &[u8], success_callback: SuccessCallback, failure_callback: FailureCallback, -) -> Result { +) -> Result { let request = connection_request::ConnectionRequest::parse_from_bytes(connection_request_bytes) - .map_err(|err| { - let msg = CString::new(err.to_string()).unwrap(); - RedisErrorFFI { - message: msg.into_raw(), - error_type: ErrorType::ConnectionError, - } + .map_err(|err| CreateClientError { + message: err.to_string(), + error_type: RequestErrorType::Unspecified, })?; let runtime = Builder::new_multi_thread() .enable_all() .thread_name("GLIDE for Redis Go thread") .build() .map_err(|err| { - let msg = CString::new(err.to_string()).unwrap(); - RedisErrorFFI { - message: msg.into_raw(), - error_type: ErrorType::ConnectionError, + let redis_error = err.into(); + CreateClientError { + message: errors::error_message(&redis_error), + error_type: errors::error_type(&redis_error), } })?; let _runtime_handle = runtime.enter(); - let client = runtime.block_on(GlideClient::new(request)).map_err(|err| { - let msg = CString::new(err.to_string()).unwrap(); - RedisErrorFFI { - message: msg.into_raw(), - error_type: ErrorType::ConnectionError, - } - })?; + let client = runtime + .block_on(GlideClient::new(request)) + .map_err(|err| CreateClientError { + message: err.to_string(), + error_type: RequestErrorType::Disconnect, + })?; Ok(Client { client, success_callback, @@ -117,11 +107,13 @@ pub unsafe extern "C" fn create_client( let response = match create_client_internal(request_bytes, success_callback, failure_callback) { Err(err) => ConnectionResponse { conn_ptr: std::ptr::null(), - error: Box::into_raw(Box::new(err)) as *const RedisErrorFFI, + error_message: CString::into_raw(CString::new(err.message).unwrap()), + error_type: err.error_type, }, Ok(client) => ConnectionResponse { conn_ptr: Box::into_raw(Box::new(client)) as *const c_void, - error: std::ptr::null(), + error_message: std::ptr::null(), + error_type: RequestErrorType::Unspecified, }, }; Box::into_raw(Box::new(response)) @@ -155,18 +147,14 @@ pub unsafe extern "C" fn free_connection_response( drop(connection_response); } -/// Deallocates a `RedisErrorFFI`. +/// Deallocates an error message `CString`. /// /// # Safety /// -/// * `error_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). -/// * The error message must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). -/// * `error_ptr` must not be null. -/// * The error message pointer must not be null. +/// * `error_msg_ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). +/// * `error_msg_ptr` must not be null. #[no_mangle] -pub unsafe extern "C" fn free_error(error_ptr: *const RedisErrorFFI) { - let error = unsafe { Box::from_raw(error_ptr as *mut RedisErrorFFI) }; - let error_msg = unsafe { CString::from_raw(error.message as *mut c_char) }; - drop(error); +pub unsafe extern "C" fn free_error(error_msg_ptr: *const c_char) { + let error_msg = unsafe { CString::from_raw(error_msg_ptr as *mut c_char) }; drop(error_msg); } From 926e0505af18f32c339d42355afffcafb10edd6d Mon Sep 17 00:00:00 2001 From: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com> Date: Wed, 6 Mar 2024 17:49:46 -0800 Subject: [PATCH 17/25] Update go/glide/lib.h Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com> --- go/glide/lib.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/glide/lib.h b/go/glide/lib.h index 3317c47a9c..019489f0dc 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -69,7 +69,7 @@ void close_client(const void *client_ptr); * * # Safety * - * * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). + * * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). * * `connection_response_ptr` must not be null. */ void free_connection_response(const struct ConnectionResponse *connection_response_ptr); From b9e61c89ce82425aafeac1290bff50d98f4d17b7 Mon Sep 17 00:00:00 2001 From: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com> Date: Wed, 6 Mar 2024 17:49:56 -0800 Subject: [PATCH 18/25] Update go/glide/lib.h Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com> --- go/glide/lib.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/glide/lib.h b/go/glide/lib.h index 019489f0dc..83074a573a 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -17,7 +17,7 @@ typedef enum RequestErrorType { /** * The connection response. * - * It contains either a connection or an error. It is represented as a struct instead of an enum for ease of use in the wrapper language. + * It contains either a connection or an error. It is represented as a struct instead of a union for ease of use in the wrapper language. */ typedef struct ConnectionResponse { const void *conn_ptr; From 70fb74b0c5aa402c66ec37d036a4a8523b5d3c49 Mon Sep 17 00:00:00 2001 From: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com> Date: Wed, 6 Mar 2024 17:50:52 -0800 Subject: [PATCH 19/25] Update go/src/lib.rs Co-authored-by: Yury-Fridlyand --- go/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/src/lib.rs b/go/src/lib.rs index 76194eb96e..948f6e23ca 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -87,7 +87,7 @@ fn create_client_internal( }) } -/// Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. +/// Creates a new client with the given configuration. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. /// /// The returned `ConnectionResponse` should be manually freed by calling `free_connection_response`, otherwise a memory leak will occur. It should be freed whether or not an error occurs. /// From db8959b7614a6ee71fd909dc12d37e1bbd98ddfa Mon Sep 17 00:00:00 2001 From: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com> Date: Wed, 6 Mar 2024 17:51:34 -0800 Subject: [PATCH 20/25] Update go/src/lib.rs Co-authored-by: Aaron <69273634+aaron-congo@users.noreply.github.com> --- go/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/src/lib.rs b/go/src/lib.rs index 948f6e23ca..5e887dfff9 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -136,7 +136,7 @@ pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { /// /// # Safety /// -/// * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). +/// * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). /// * `connection_response_ptr` must not be null. #[no_mangle] pub unsafe extern "C" fn free_connection_response( From b32408c2c2833848c2f144137aa321590901e1d0 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Wed, 6 Mar 2024 17:55:17 -0800 Subject: [PATCH 21/25] Update documentation for create_client --- go/glide/lib.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/glide/lib.h b/go/glide/lib.h index 83074a573a..54ccc3b73c 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -40,7 +40,7 @@ typedef void (*FailureCallback)(uintptr_t channel_address, RequestErrorType error_type); /** - * Creates a new client to the given address. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. + * Creates a new client with the given configuration. The success callback needs to copy the given string synchronously, since it will be dropped by Rust once the callback returns. All callbacks should be offloaded to separate threads in order not to exhaust the client's thread pool. * * The returned `ConnectionResponse` should be manually freed by calling `free_connection_response`, otherwise a memory leak will occur. It should be freed whether or not an error occurs. * From 478acd6a26724101a2b208e1b9c8be8ebdb11f40 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Thu, 7 Mar 2024 10:24:51 -0800 Subject: [PATCH 22/25] Adjust documentation regarding memory leaks --- go/glide/lib.h | 6 +++++- go/src/lib.rs | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/go/glide/lib.h b/go/glide/lib.h index 54ccc3b73c..1cece94da7 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -17,7 +17,9 @@ typedef enum RequestErrorType { /** * The connection response. * - * It contains either a connection or an error. It is represented as a struct instead of a union for ease of use in the wrapper language. + * It contains either a connection or an error. It is represented as a struct instead of an enum for ease of use in the wrapper language. + * + * This struct should be freed using both `free_connection_response` and `free_error` to avoid memory leaks. */ typedef struct ConnectionResponse { const void *conn_ptr; @@ -67,6 +69,8 @@ void close_client(const void *client_ptr); /** * Deallocates a `ConnectionResponse`. * + * This function does not free the contained error, which needs to be freed separately using `free_error` afterwards to avoid memory leaks. + * * # Safety * * * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). diff --git a/go/src/lib.rs b/go/src/lib.rs index 5e887dfff9..89aaadb8bd 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -30,6 +30,8 @@ pub type FailureCallback = unsafe extern "C" fn( /// The connection response. /// /// It contains either a connection or an error. It is represented as a struct instead of an enum for ease of use in the wrapper language. +/// +/// This struct should be freed using both `free_connection_response` and `free_error` to avoid memory leaks. #[repr(C)] pub struct ConnectionResponse { conn_ptr: *const c_void, @@ -134,6 +136,8 @@ pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { /// Deallocates a `ConnectionResponse`. /// +/// This function does not free the contained error, which needs to be freed separately using `free_error` afterwards to avoid memory leaks. +/// /// # Safety /// /// * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). From 0e52d914e3492bb4cea8fe90d09f95c7e3860142 Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Thu, 7 Mar 2024 10:47:34 -0800 Subject: [PATCH 23/25] Make free_connection_response also free its error --- go/glide/glide.go | 1 - go/glide/lib.h | 11 +---------- go/src/lib.rs | 8 ++++++-- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/go/glide/glide.go b/go/glide/glide.go index ae16cb0ecf..5f486de808 100644 --- a/go/glide/glide.go +++ b/go/glide/glide.go @@ -60,7 +60,6 @@ func (glideRedisClient *GlideRedisClient) ConnectToRedis(request *protobuf.Conne response := (*C.struct_ConnectionResponse)(C.create_client((*C.uchar)(requestBytes), C.uintptr_t(byteCount), (C.SuccessCallback)(unsafe.Pointer(C.successCallback)), (C.FailureCallback)(unsafe.Pointer(C.failureCallback)))) defer C.free_connection_response(response) if response.error_message != nil { - defer C.free_error(response.error_message) return fmt.Errorf(C.GoString(response.error_message)) } glideRedisClient.coreClient = response.conn_ptr diff --git a/go/glide/lib.h b/go/glide/lib.h index 1cece94da7..e468c600e2 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -75,15 +75,6 @@ void close_client(const void *client_ptr); * * * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). * * `connection_response_ptr` must not be null. + * * The contained `error_message` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). */ void free_connection_response(const struct ConnectionResponse *connection_response_ptr); - -/** - * Deallocates an error message `CString`. - * - * # Safety - * - * * `error_msg_ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). - * * `error_msg_ptr` must not be null. - */ -void free_error(const char *error_msg_ptr); diff --git a/go/src/lib.rs b/go/src/lib.rs index 89aaadb8bd..44c31d02a7 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -142,13 +142,18 @@ pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { /// /// * `connection_response_ptr` must be able to be safely casted to a valid `Box` via `Box::from_raw`. See the safety documentation of [`std::boxed::Box::from_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.from_raw). /// * `connection_response_ptr` must not be null. +/// * The contained `error_message` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). #[no_mangle] pub unsafe extern "C" fn free_connection_response( connection_response_ptr: *const ConnectionResponse, ) { let connection_response = unsafe { Box::from_raw(connection_response_ptr as *mut ConnectionResponse) }; + let error_message = connection_response.error_message; drop(connection_response); + if error_message != std::ptr::null() { + free_error(error_message); + } } /// Deallocates an error message `CString`. @@ -157,8 +162,7 @@ pub unsafe extern "C" fn free_connection_response( /// /// * `error_msg_ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). /// * `error_msg_ptr` must not be null. -#[no_mangle] -pub unsafe extern "C" fn free_error(error_msg_ptr: *const c_char) { +unsafe fn free_error(error_msg_ptr: *const c_char) { let error_msg = unsafe { CString::from_raw(error_msg_ptr as *mut c_char) }; drop(error_msg); } From a02912f56c55d1d006c6967eea69a0b5709e57af Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Fri, 8 Mar 2024 13:31:38 -0800 Subject: [PATCH 24/25] Update doc comment for free_connection_response --- go/glide/lib.h | 2 +- go/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/go/glide/lib.h b/go/glide/lib.h index e468c600e2..0fb30a750d 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -69,7 +69,7 @@ void close_client(const void *client_ptr); /** * Deallocates a `ConnectionResponse`. * - * This function does not free the contained error, which needs to be freed separately using `free_error` afterwards to avoid memory leaks. + * This function also frees the contained error using `free_error`. * * # Safety * diff --git a/go/src/lib.rs b/go/src/lib.rs index 44c31d02a7..5feca11671 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -136,7 +136,7 @@ pub unsafe extern "C" fn close_client(client_ptr: *const c_void) { /// Deallocates a `ConnectionResponse`. /// -/// This function does not free the contained error, which needs to be freed separately using `free_error` afterwards to avoid memory leaks. +/// This function also frees the contained error using `free_error`. /// /// # Safety /// From 7ea7bdd4fe44b708d2ab7b724701a418483d417d Mon Sep 17 00:00:00 2001 From: Jonathan Louie Date: Fri, 8 Mar 2024 14:11:27 -0800 Subject: [PATCH 25/25] Re-expose free_error for the error callback to use and update docs --- go/glide/lib.h | 12 +++++++++++- go/src/lib.rs | 5 +++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/go/glide/lib.h b/go/glide/lib.h index 0fb30a750d..421e9c1379 100644 --- a/go/glide/lib.h +++ b/go/glide/lib.h @@ -19,7 +19,7 @@ typedef enum RequestErrorType { * * It contains either a connection or an error. It is represented as a struct instead of an enum for ease of use in the wrapper language. * - * This struct should be freed using both `free_connection_response` and `free_error` to avoid memory leaks. + * This struct should be freed using `free_connection_response` to avoid memory leaks. */ typedef struct ConnectionResponse { const void *conn_ptr; @@ -78,3 +78,13 @@ void close_client(const void *client_ptr); * * The contained `error_message` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). */ void free_connection_response(const struct ConnectionResponse *connection_response_ptr); + +/** + * Deallocates an error message `CString`. + * + * # Safety + * + * * `error_msg_ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). + * * `error_msg_ptr` must not be null. + */ +void free_error(const char *error_msg_ptr); diff --git a/go/src/lib.rs b/go/src/lib.rs index 5feca11671..0151c482b1 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -31,7 +31,7 @@ pub type FailureCallback = unsafe extern "C" fn( /// /// It contains either a connection or an error. It is represented as a struct instead of an enum for ease of use in the wrapper language. /// -/// This struct should be freed using both `free_connection_response` and `free_error` to avoid memory leaks. +/// This struct should be freed using `free_connection_response` to avoid memory leaks. #[repr(C)] pub struct ConnectionResponse { conn_ptr: *const c_void, @@ -162,7 +162,8 @@ pub unsafe extern "C" fn free_connection_response( /// /// * `error_msg_ptr` must be able to be safely casted to a valid `CString` via `CString::from_raw`. See the safety documentation of [`std::ffi::CString::from_raw`](https://doc.rust-lang.org/std/ffi/struct.CString.html#method.from_raw). /// * `error_msg_ptr` must not be null. -unsafe fn free_error(error_msg_ptr: *const c_char) { +#[no_mangle] +pub unsafe extern "C" fn free_error(error_msg_ptr: *const c_char) { let error_msg = unsafe { CString::from_raw(error_msg_ptr as *mut c_char) }; drop(error_msg); }