エコーサーバー・クライアントの実装（追加機能） by thonda28 · Pull Request #2 · thonda28/c-echo

thonda28 · 2025-01-07T08:53:47Z

Description

#1 の続き

この PR では以下3つの機能追加を行う

大量のリクエスト処理
- epoll を使って I/O 多重化
- listen_sockets および client_sockets は SocketManager という構造体を定義して管理
IPv6 対応
- getaddrinfo() で取得した情報を使って socket を作成
- client 側も引数として与えたアドレスの形式によって、該当のプロトコルにリクエスト（127.0.0.1 なら IPv4、::1 なら IPv6）
適切な終了
- pipe を epoll に登録して、SIGINT がきたら Cleanup 処理に移行

Note

対応できてないものは以下（他にも対応できていないものはありそうだが、ひとまず自分で認識しているもの）

binary の入力
- fread() で入力できそうだったが、Interactive に入力することはできない？
struct SockData で管理している buffer のサイズが固定
- ここも動的に管理できるとよさそう

inazz · 2025-01-07T11:53:19Z

+            while ((received_bytes = recv(conn_sock, buf, sizeof(buf) - 1, 0)) > 0)
+            {
+                printf("received_bytes: %ld\n", received_bytes);
+                buf[received_bytes] = '\0'; // Null-terminate the string


received buffer が文字列である (バイナリで \0 を含んでいない) ことが前提になっている?

…nction

inazz · 2025-01-14T11:26:42Z

+    while (1)
+    {
+        char buf[BUFFER_SIZE];
+        fgets(buf, sizeof(buf), stdin); // Null-terminate the string


入力がバイト列の場合はどうする?

fgets が一番最後まで読み切ったときは break したい。

…nected_socket() function

…en_sockets() function

…g Socket Manager

…o_epoll() function

…ent side

…N or EPOLLOUT event

inazz · 2025-01-25T12:24:22Z

-run-client:build-client
-	./client
+server: server.o utils.o
+	$(CC) $(CFLAGS) -o server server.o utils.o


$@
$^

という変数もあります

inazz · 2025-01-25T13:35:30Z

    {
-        perror("client: connect()");
-        close(sock);
+        puts("Failed to create a connected socket\n");


perror は stderr に表示されるので、合わせてこれも stderr に出力した方が良いと思います。

inazz · 2025-01-25T13:50:23Z

+    struct sockaddr_in6 server_addr6;
+    memset(&server_addr4, 0, sizeof(server_addr4));
+    memset(&server_addr6, 0, sizeof(server_addr6));
+    int is_ipv4 = inet_pton(PF_INET, ip, &server_addr4.sin_addr);


is_xxxxx という名前は true or false を連想させるので違う名前にしたい。 (そのままif 文で評価して -1 を true 扱いしてしまう事故が発生しそう)

inazz · 2025-01-25T13:53:23Z

+    int socket_fd;
+    if (is_ipv4 == 1)
+    {
+        if ((socket_fd = socket(PF_INET, SOCK_STREAM, 0)) == -1)


パラメータが違う同じコードが2か所(v4/v6)にあるので、共通化したい。

inazz · 2025-01-26T00:48:42Z

+
+#include "utils.h"
+
+void init_socket_manager(SocketManager *manager, int max_size)


メモリ確保が init_socket_manager で
解放が close_all_sockets で対で呼ぶ必要があるのは、名前からだと読み解くのが難しいです。

SocketManager *new_socket_manager(int max_size)
void free_socket_manager(SocketManager *)

などとするのはいかがでしょうか?

inazz · 2025-01-27T03:29:44Z

+        ssize_t sent_bytes;
+        if ((sent_bytes = send(client_socket_data->socket_fd, client_socket_data->buffer, strlen(client_socket_data->buffer), 0)) >= 0)
+        {
+            memmove(client_socket_data->buffer, client_socket_data->buffer + sent_bytes, strlen(client_socket_data->buffer) - sent_bytes);


データのコピーにデータ量に時間がかかるためmemmove は避けたい
(send にバッファの途中のポインタを与えたい)

inazz · 2025-01-27T03:30:40Z

+    }
+
+    // Check if the client socket is ready to write
+    if (event.events & EPOLLOUT)


送るデータが無かったら? (バッファが0の時に EPOLLOUT を受信しない)

inazz · 2025-01-27T03:32:22Z

+        if ((received_bytes = recv(client_socket_data->socket_fd, client_socket_data->buffer, BUFFER_SIZE - 1, 0)) > 0)
+        {
+            printf("received_bytes: %ld (fd: %d)\n", received_bytes, client_socket_data->socket_fd);
+            client_socket_data->buffer[received_bytes] = '\0'; // Null-terminate the string


外からやってくるデータがバイナリデータ (\0を含む)かもしれないので、データがどこまで存在しているかを \0 に依存したくない。
(strlen は O(N) 時間がかかるのもあります)
client_socket_data にバッファに何文字入っているかを保存させては?

inazz · 2025-01-27T03:34:03Z

+    }
+
+    struct epoll_event event;
+    event.events = EPOLLIN | EPOLLOUT;


送り返すデータが存在しない場合に EPOLLOUT を受け取ってもやることがない (= 無限に空パケットを送ったり CPU を無駄にするループになる可能性)

ここでおっしゃってるのは、そもそも EPOLLOUT を受け取らないように修正する、ということですかね？（#3 では送り返すデータが存在しなければ処理を終えるように修正しました）
もし上記の意図だとすると、バッファの状況を見ながら epoll_event の監視をやめたり始めたりとかそんな感じでしょうか？

inazz · 2025-01-27T03:39:18Z

+                }
+            }
+            // Check if the event is for a client socket
+            else if ((socket_data = find_socket(&client_socket_manager, events[i].data.fd)) != NULL)


find_socket は O(N) かかり、 epoll 自体がソケットの数に比例する時間を必要としないように作られている利点を消してしまっています。
epoll_event->data を epollの設定時にセットすることで、イベントの受信時に受け取ることができるので、ここに(data->ptrなどに) SocketData を保持するのはいかがでしょうか?
(close時も O(1)にすることを考えるとSocketData を少し拡張する必要があるかもしれません)

handle_new_connection の分岐も同様

提案ありがとうございます。epoll_event->data->ptr の存在に気づいていなかったですが、たしかにここを活用できますね。SocketData のポインタをもたせることで socket_manager を線形探索せずに SocketData を取得できるよう #3 で修正してみました。イメージ合っていそうでしょうか？

ただ削除時の実装まではできていないです。（断念しました）
実装するとすると、SocketManager では現状配列で SocketData を管理していますが、ここをハッシュテーブルにして、キーを fd に、バリューに SocketData のポインタを持たせるとかですかね？

thonda28 · 2025-01-27T04:41:57Z

@inazz
レビューおよびたくさんのコメントありがとうございます、勉強になります！
この PR のまま修正するとややこしく感じるので一度この PR をマージして、コメントいただいたものについては別 PR で対応しようと思います！

thonda28

@inazz
レビューありがとうございました！いただいたコメントに対する修正自体は #3 で行いましたが、意図を汲めていないかもと思う部分が3点あったのでそれはそのままこちらの PR で返信させていただきました。

#2 でも #3 でも返信がしやすい方で、お時間あるときにご回答いただけると助かります。（#3 の Description にも同様のことを書いています）

thonda28 · 2025-02-06T06:36:52Z

+                }
+            }
+            // Check if the event is for a client socket
+            else if ((socket_data = find_socket(&client_socket_manager, events[i].data.fd)) != NULL)


提案ありがとうございます。epoll_event->data->ptr の存在に気づいていなかったですが、たしかにここを活用できますね。SocketData のポインタをもたせることで socket_manager を線形探索せずに SocketData を取得できるよう #3 で修正してみました。イメージ合っていそうでしょうか？

ただ削除時の実装まではできていないです。（断念しました）
実装するとすると、SocketManager では現状配列で SocketData を管理していますが、ここをハッシュテーブルにして、キーを fd に、バリューに SocketData のポインタを持たせるとかですかね？

thonda28 · 2025-02-06T06:41:16Z

+    }
+
+    struct epoll_event event;
+    event.events = EPOLLIN | EPOLLOUT;


ここでおっしゃってるのは、そもそも EPOLLOUT を受け取らないように修正する、ということですかね？（#3 では送り返すデータが存在しなければ処理を終えるように修正しました）
もし上記の意図だとすると、バッファの状況を見ながら epoll_event の監視をやめたり始めたりとかそんな感じでしょうか？

thonda28 · 2025-02-06T06:41:55Z

+    {
+        if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR)
+        {
+            continue;


ここちょっとわからなかったです。
EWOULDBLOCK の場合にも書き込み自体は発生していて、パイプのバッファサイズ上限に達するまで書き込み続けてしまう、みたいな感じでしょうか。EWOULDBLOCK が起きるケースのイメージがあまりついておらず、どんな動きになるかがわかっていないです。

thonda28 added 5 commits January 7, 2025 02:00

Add IPv6 support to server

947bce1

Modify client to maintain connection after sending data

51e3ec6

Handle server disconnection by closing client connection and exiting

c7bf378

Implement I/O multiplexing using select()

bf46092

Close socket when max_clients limit is exceeded

1835e32

inazz reviewed Jan 7, 2025

View reviewed changes

Comment thread server.c Outdated

Comment thread server.c Outdated

thonda28 added 11 commits January 10, 2025 16:07

Modify server's main function to accept port as an argument

cb52c2f

Extract function declarations to utils.h

f5f3534

Modify Client to use parse_port() from utils.h

36f673c

Address review comments

c0af539

Extract client handling logic into handle_client() function

e63117e

Define BUFFER_SIZE

71f16b2

Use epoll() instead of select()

6d6bd01

Use stack to manage client sockets efficiently

1aa9a74

Extract new connection handling logic into handle_new_connection() fu…

d726454

…nction

Declare function signature before main

3b1365d

Add support for both IPv4 and IPv6

3554dc5

inazz reviewed Jan 14, 2025

View reviewed changes

thonda28 added 11 commits January 15, 2025 08:51

Modify client to connect both IPv4 and IPv6 address

435b1d0

Extract socket creation and connection handling logic into create_con…

43d73ed

…nected_socket() function

Modify parse_port() to return int type

388a516

Use BUFFER_SIZE instead of sizeof(buf)

1667b66

Fix some minors

a4982b7

Modify handle_new_connection() to return int value

4dc05d0

Modify handle_client() to return int value

f333a30

Extract socket creation and listening handling logic into create_list…

fabe42c

…en_sockets() function

Manage client_sockets using SocketManager

0504445

Refactor to support more than two listen_sockets and manage them usin…

94607c9

…g Socket Manager

Use contains for simplicity

78d925d

thonda28 added 14 commits January 16, 2025 09:24

Remove redundant parts from Makefile

adcb7f9

Fix some minors

a6865bf

Organize error handlings

506020c

Add detailed documentation comments for all functions

8d4013a

Extract listening socket registration logic into add_listen_sockets_t…

1e1e22e

…o_epoll() function

Modify server error handling not to exit 1 when error occuring on cli…

442fe0a

…ent side

Refactor SocketManager to dynamically manage the maximum size

03074e4

Refactor error handling to use goto for common cleanup

634519e

Modify handle_client to execute recv() or send() only once per EPOLLI…

ccf3478

…N or EPOLLOUT event

Modify client to ensure complete send and receive of data

2e313ca

Use fwrite() instead of printf()

1632d0f

Add error handling about EINTR

0232176

Implement close_with_retry() instead of close() to deal with EINTR

f7f6b1f

Monitor signal to shutdown after cleanup

9f70e15

thonda28 marked this pull request as ready for review January 17, 2025 10:30

inazz reviewed Jan 27, 2025

View reviewed changes

thonda28 merged commit bb1c3ea into main Jan 30, 2025

thonda28 deleted the feature/additional-functions branch February 5, 2025 03:15

thonda28 mentioned this pull request Feb 5, 2025

エコーサーバー・クライアントの実装（いただいたコメントの対応） #3

Open

thonda28 commented Feb 6, 2025

View reviewed changes


		#include "utils.h"

		void init_socket_manager(SocketManager *manager, int max_size)

Conversation

thonda28 commented Jan 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Note

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thonda28 Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thonda28 commented Jan 27, 2025

Uh oh!

thonda28 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thonda28 Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thonda28 commented Jan 7, 2025 •

edited

Loading

thonda28 Feb 6, 2025 •

edited

Loading

thonda28 left a comment •

edited

Loading

thonda28 Feb 6, 2025 •

edited

Loading