Merge pull request #46

Windows builds

.github/workflows/build.yaml

@@ -104,37 +104,57 @@ jobs:
           cmake --build .build --config Release
 
   windows:
-    # Windows builds are broken at the moment, disable until they are fixed
-    if: false
     runs-on: windows-latest
     strategy:
       fail-fast: false
       matrix:
+        compiler:
+          - cl
         variant:
-          - libuv
+          # [Nov. 2024] msvc throws C2894 errors when using libuv, need to investigate why
+          # Ref: https://github.com/uatuko/grpcxx/pull/46
+          # - libuv
           - asio
         include:
           - variant: asio
             cmake_args: -DGRPCXX_USE_ASIO=ON
 
     steps:
-      - uses: actions/cache@v4
+      - uses: actions/cache/restore@v4
+        id: cache-restore-vcpkg
         with:
           path: C:\vcpkg\packages
           key: ${{ runner.arch }}_${{ runner.os }}-vcpkg-20241101
-      - name: Install dependencies
-        run: |
-          choco install ninja
-          vcpkg install protobuf
+
+      - name: Install dependencies (choco)
+        run: choco install ninja
+      - name: Install dependencies (vcpkg)
+        if: steps.cache-restore-vcpkg.outputs.cache-hit != 'true'
+        run: vcpkg install protobuf
+
+      # Always save vcpkg cache if vcpkg install is successful
+      - uses: actions/cache/save@v4
+        if: always() && steps.cache-restore-vcpkg.outputs.cache-hit != 'true'
+        id: cache-save-vcpkg
+        with:
+          path: C:\vcpkg\packages
+          key: ${{ steps.cache-restore-vcpkg.outputs.cache-primary-key }}
+
+      # It's not straight forward to setup dev commant prompt to use across steps. Have to find the location and execute
+      # `vcvarsall.bat ${{ runner.arch }}` and "export" env vars set by vcvarsall.bat.
+      - uses: ilammy/msvc-dev-cmd@v1
+
       - uses: actions/checkout@v4
       - name: Configure CMake
         run: |
           cmake -B .build -G Ninja `
+            -DCMAKE_CXX_COMPILER=${{ matrix.compiler }} `
             -DCMAKE_BUILD_TYPE=Release `
             -DGRPCXX_BUILD_EXAMPLES=OFF `
             -DGRPCXX_BUILD_EXPERIMENTS=OFF `
             -DGRPCXX_BUILD_TESTING=OFF `
             -DProtobuf_ROOT=C:\vcpkg\packages\protobuf_x64-windows `
+            -DCMAKE_CXX_FLAGS="-DNGHTTP2_NO_SSIZE_T -IC:\vcpkg\packages\abseil_x64-windows\include" `
             ${{ matrix.cmake_args }}
       - name: Build
         run: cmake --build .build --config Release

CMakeLists.txt

@@ -9,6 +9,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 set(EXECUTABLE_OUTPUT_PATH ${CMAKE_BINARY_DIR}/bin)
 
+option(BUILD_SHARED_LIBS "Build grpcxx as a shared library" ON)
 option(GRPCXX_HERMETIC_BUILD "Fetch and build all dependencies instead of relying on system libraries" ON)
 option(GRPCXX_USE_ASIO "Use asio instead of libuv for I/O and event loop" OFF)
 
@@ -19,13 +20,13 @@ cmake_dependent_option(GRPCXX_BUILD_EXAMPLES
 )
 
 cmake_dependent_option(GRPCXX_BUILD_EXPERIMENTS
-	"Build experiments when this is the root project" ON
+	"Build experiments when this is the root project and not using asio" ON
 	"NOT GRPCXX_USE_ASIO; CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR" OFF
 )
 
 cmake_dependent_option(GRPCXX_BUILD_TESTING
-	"Build tests when BUILD_TESTING flag is set and this is the root project" ON
-	"BUILD_TESTING; CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR" OFF
+	"Build tests when BUILD_TESTING flag is set or this is the root project" ON
+	"BUILD_TESTING OR CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR" OFF
 )
 
 include(cmake/dependencies.cmake)

cmake/dependencies.cmake

@@ -2,7 +2,7 @@ include(FetchContent)
 
 set(LIBUV_MINVERSION 1.46.0)
 set(BOOST_MINVERSION 1.81)
-set(NGHTTP2_MINVERSION 1.55.1)
+set(NGHTTP2_MINVERSION 1.64.0)
 set(PROTOBUF_MINVERSION 3.15.0)
 set(FMT_MINVERSION 10.1.1)
 set(GTEST_MINVERSION 1.15.2)
@@ -15,10 +15,16 @@ if(NOT GRPCXX_USE_ASIO)
             URL_HASH SHA256=7aa66be3413ae10605e1f5c9ae934504ffe317ef68ea16fdaa83e23905c681bd
         )
 
-        set(LIBUV_BUILD_SHARED OFF CACHE BOOL "Build libuv shared lib")
+        set(LIBUV_BUILD_SHARED ${BUILD_SHARED_LIBS} CACHE BOOL "Build libuv shared lib")
         FetchContent_MakeAvailable(libuv)
-        install(TARGETS uv_a EXPORT grpcxx COMPONENT Development)
-        add_library(libuv::uv ALIAS uv_a)
+
+        if (BUILD_SHARED_LIBS)
+            install(TARGETS uv EXPORT grpcxx COMPONENT Development)
+            add_library(libuv::uv ALIAS uv)
+        else()
+            install(TARGETS uv_a EXPORT grpcxx COMPONENT Development)
+            add_library(libuv::uv ALIAS uv_a)
+        endif()
     else()
         # Unfortunately the libuv CMakeLists.txt does not export
         # a version file. This is a bug in libuv.
@@ -100,22 +106,23 @@ if(NOT GRPCXX_HERMETIC_BUILD)
 else()
     FetchContent_Declare(nghttp2
         URL      https://github.com/nghttp2/nghttp2/releases/download/v${NGHTTP2_MINVERSION}/nghttp2-${NGHTTP2_MINVERSION}.tar.xz
-        URL_HASH SHA256=19490b7c8c2ded1cf7c3e3a54ef4304e3a7876ae2d950d60a81d0dc6053be419
+        URL_HASH SHA256=88bb94c9e4fd1c499967f83dece36a78122af7d5fb40da2019c56b9ccc6eb9dd
     )
 
+    if (NOT BUILD_SHARED_LIBS)
+        set(BUILD_STATIC_LIBS ON CACHE BOOL "Build libnghttp2 in static mode")
+    endif()
     set(ENABLE_LIB_ONLY   ON  CACHE BOOL "Build libnghttp2 only")
-    set(ENABLE_STATIC_LIB ON  CACHE BOOL "Build libnghttp2 in static mode")
-    set(ENABLE_SHARED_LIB OFF CACHE BOOL "Build libnghttp2 as a shared library")
     set(ENABLE_DOC        OFF CACHE BOOL "Build libnghttp2 documentation")
     FetchContent_MakeAvailable(nghttp2)
 
-    target_include_directories(nghttp2_static
-        PUBLIC
-            $<BUILD_INTERFACE:${nghttp2_SOURCE_DIR}/lib/includes>
-    )
-
-    install(TARGETS nghttp2_static EXPORT grpcxx COMPONENT Development)
-    add_library(libnghttp2::nghttp2 ALIAS nghttp2_static)
+    if (BUILD_SHARED_LIBS)
+        install(TARGETS nghttp2 EXPORT grpcxx COMPONENT Development)
+        add_library(libnghttp2::nghttp2 ALIAS nghttp2)
+    else()
+        install(TARGETS nghttp2_static EXPORT grpcxx COMPONENT Development)
+        add_library(libnghttp2::nghttp2 ALIAS nghttp2_static)
+    endif()
 endif()
 
 # protobuf

docs/api.md

@@ -1,7 +1,7 @@
 # API Documentation
 
 > [!WARNING]
-> `grpcxx::detail` namespace is considered internal and may change without any warning.
+> `*::detail` namespaces are considered internal and may change without any warning.
 
 > [!IMPORTANT]
 > (`asio`) indicates it's only available when using Asio (i.e. compiled with `GRPCXX_USE_ASIO`).
@@ -170,16 +170,16 @@ _awaitable_ must be passed on to an `io_context` executor to serve requests.
 ------------------------------------------------- | ---
 `void run(const std::string_view &ip, int port);`                           | (1) (`asio`)
 `void run(std::string_view ip, int port, std::stop_token stop_token = {});` | (2) (`libuv`)
-`void run(int fd, std::stop_token stop_token = {});`                        | (3) (`libuv`)
+`void run(uv_os_sock_t sock, std::stop_token stop_token = {});`             | (3) (`libuv`)
 
 Listen and serve incoming gRPC requests.
 
 1. Start listening on `ip` and `port` for incoming gRPC connections and serve requests.
 2. Same as (1), but accepting an optional stop token to asynchronously signal the server to exit.
-3. Start listening on the provided file descriptor `fd`, which needs to be already bound to a network 
-   address. This is useful when the socket needs to have some additional properties set (such as 
-   keep-alive) and/or reused from the outside run context (such as is the case of the 
-   [systemd socket activation protocol](https://www.freedesktop.org/software/systemd/man/latest/sd_listen_fds.html#)).
+3. Start listening on the provided tcp socket `sock`, which needs to be already bound to a network
+   address. This is useful when the socket needs to have some additional properties set (such as
+   keep-alive) and/or reused from the outside run context (such as is the case of the
+   [systemd socket activation protocol](https://www.freedesktop.org/software/systemd/man/latest/sd_listen_fds.html)).
 
 > [!IMPORTANT]
 > If used with Asio, this will create and run an `io_context` executor on the main thread.

lib/grpcxx/h2/session.cpp

@@ -40,15 +40,15 @@ session::~session() {
 
 void session::data(int32_t stream_id, std::string &&data) {
 	_data = std::move(data);
-	nghttp2_data_provider provider{
+	nghttp2_data_provider2 provider{
 		.source =
 			{
 				.ptr = &_data,
 			},
 		.read_callback = read_cb,
 	};
 
-	if (auto r = nghttp2_submit_data(_session, NGHTTP2_FLAG_NONE, stream_id, &provider); r != 0) {
+	if (auto r = nghttp2_submit_data2(_session, NGHTTP2_FLAG_NONE, stream_id, &provider); r != 0) {
 		throw std::runtime_error(std::string("Failed to submit data: ") + nghttp2_strerror(r));
 	}
 }
@@ -126,7 +126,7 @@ void session::headers(int32_t stream_id, detail::headers hdrs) const {
 
 std::string_view session::pending() {
 	const uint8_t *bytes;
-	auto           n = nghttp2_session_mem_send(_session, &bytes);
+	auto           n = nghttp2_session_mem_send2(_session, &bytes);
 	if (n < 0) {
 		throw std::runtime_error(
 			std::string("Failed to retrieve pending session data: ") + nghttp2_strerror(n));
@@ -136,7 +136,7 @@ std::string_view session::pending() {
 }
 
 session::events_t session::read(std::string_view bytes) {
-	if (auto n = nghttp2_session_mem_recv(
+	if (auto n = nghttp2_session_mem_recv2(
 			_session, reinterpret_cast<const uint8_t *>(bytes.data()), bytes.size());
 		n < 0) {
 		throw std::runtime_error(
@@ -149,7 +149,7 @@ session::events_t session::read(std::string_view bytes) {
 	return events;
 }
 
-ssize_t session::read_cb(
+nghttp2_ssize session::read_cb(
 	nghttp2_session *session, int32_t stream_id, uint8_t *buf, size_t length, uint32_t *data_flags,
 	nghttp2_data_source *source, void *) {
 	auto *str = static_cast<std::string *>(source->ptr);

lib/grpcxx/h2/session.h

@@ -38,7 +38,7 @@ class session {
 		nghttp2_session *session, const nghttp2_frame *frame, const uint8_t *name, size_t namelen,
 		const uint8_t *value, size_t valuelen, uint8_t flags, void *vsess);
 
-	static ssize_t read_cb(
+	static nghttp2_ssize read_cb(
 		nghttp2_session *session, int32_t stream_id, uint8_t *buf, size_t length,
 		uint32_t *data_flags, nghttp2_data_source *source, void *);
 

lib/grpcxx/uv/server.cpp

@@ -2,10 +2,6 @@
 
 #include "conn.h"
 #include "coroutine.h"
-#include "uv.h"
-
-#include <netinet/in.h>
-#include <sys/socket.h>
 
 #include <stdexcept>
 #include <stop_token>
@@ -72,16 +68,22 @@ void server::prepare(std::string_view ip, int port) {
 	start_listening();
 }
 
-void server::prepare(int fd) {
-	struct sockaddr_in addr;
-	socklen_t          addr_len = sizeof(addr);
-	if (auto r = uv_tcp_open(&_handle, fd);
-		r != 0 || getsockname(fd, (struct sockaddr *)&addr, &addr_len) != 0) {
+void server::prepare(uv_os_sock_t sock) {
+	if (auto r = uv_tcp_open(&_handle, sock); r != 0) {
+		throw std::runtime_error(
+			std::string("Failed to open socket as a tcp handle: ") + uv_strerror(r));
+	}
 
+#ifndef WIN32
+	// libuv windows implementation already checks the socket address family and calls
+	// uv_tcp_getsockname() within uv_tcp_open(), no need to check again.
+	sockaddr_storage name;
+	int              namelen = sizeof(name);
+	if (auto r = uv_tcp_getsockname(&_handle, (sockaddr *)&name, &namelen); r != 0) {
 		throw std::runtime_error(
-			std::string("Provided fd ") + std::to_string(fd) +
-			" is not a bound network socket: " + uv_strerror(r));
+			std::string("Failed to retrieve bound address for socket: ") + uv_strerror(r));
 	}
+#endif
 
 	start_listening();
 }
@@ -100,8 +102,8 @@ void server::run(std::string_view ip, int port, std::stop_token stop_token) {
 	uv_run(_loop, UV_RUN_DEFAULT);
 }
 
-void server::run(int fd, std::stop_token stop_token) {
-	prepare(fd);
+void server::run(uv_os_sock_t sock, std::stop_token stop_token) {
+	prepare(sock);
 	setup_stop_timer(std::move(stop_token));
 	uv_run(_loop, UV_RUN_DEFAULT);
 }

lib/grpcxx/uv/server.h

@@ -36,7 +36,7 @@ class server : public ::grpcxx::server_base {
 	/// Please note that this will take ownership of the file
 	/// descriptor, and close it as necessary.
 	///
-	void run(int fd, std::stop_token stop_token = {});
+	void run(uv_os_sock_t sock, std::stop_token stop_token = {});
 
 	void run(std::string_view ip, int port, std::stop_token stop_token = {});
 
@@ -56,7 +56,7 @@ class server : public ::grpcxx::server_base {
 	/// This is useful for integration in other event loops
 	/// (or nesting `uv_loop_t`s).
 	///
-	void prepare(int fd);
+	void prepare(uv_os_sock_t sock);
 
 	/// Lower-level API for integration in other event loops
 	///