Skip to content

Conversation

@zhhyu7
Copy link
Contributor

@zhhyu7 zhhyu7 commented Dec 30, 2025

Summary

remove the use of net_lock in the local module and decouple it from other network modules.

Impact

net::local

Testing

sim:matter with test code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/wait.h>

#define SOCKET_PATH "local_socket"
#define BUFFER_SIZE 1024

// Cleanup function: delete socket file
static void cleanup_socket(const char *path) {
    if (unlink(path) == -1 && errno != ENOENT) {
        perror("Failed to delete socket file");
    }
}

// Server function
static int run_server(void) {
    int server_fd, client_fd;
    struct sockaddr_un server_addr, client_addr;
    socklen_t client_len;
    char buffer[BUFFER_SIZE];
    ssize_t bytes;
    
    printf("[Server] Creating PF_UNIX SOCK_STREAM socket...\n");
    server_fd = socket(PF_UNIX, SOCK_STREAM, 0);
    if (server_fd < 0) {
        perror("Failed to create server socket");
        return -1;
    }
    
    // Clean up old socket file if it exists
    cleanup_socket(SOCKET_PATH);
    
    // Set up server address
    memset(&server_addr, 0, sizeof(server_addr));
    server_addr.sun_family = AF_UNIX;
    strncpy(server_addr.sun_path, SOCKET_PATH, sizeof(server_addr.sun_path) - 1);
    
    printf("[Server] Binding to path: %s\n", SOCKET_PATH);
    if (bind(server_fd, (struct sockaddr *)&server_addr, sizeof(server_addr)) < 0) {
        perror("Bind failed");
        close(server_fd);
        return -1;
    }
    
    printf("[Server] Starting to listen for connections...\n");
    if (listen(server_fd, 5) < 0) {
        perror("Listen failed");
        close(server_fd);
        return -1;
    }
    
    // Accept client connection
    client_len = sizeof(client_addr);
    printf("[Server] Waiting for client connection...\n");
    client_fd = accept(server_fd, (struct sockaddr *)&client_addr, &client_len);
    if (client_fd < 0) {
        perror("Accept failed");
        close(server_fd);
        return -1;
    }
    printf("[Server] Client connected\n");
    
    // Receive message from client
    memset(buffer, 0, BUFFER_SIZE);
    bytes = recv(client_fd, buffer, BUFFER_SIZE - 1, 0);
    if (bytes < 0) {
        perror("Failed to receive message");
        close(client_fd);
        close(server_fd);
        return -1;
    }
    printf("[Server] Received message from client: %s\n", buffer);
    
    // Send response to client
    const char *response = "Hello client, I am the server!";
    printf("[Server] Sending response: %s\n", response);
    if (send(client_fd, response, strlen(response), 0) < 0) {
        perror("Failed to send response");
        close(client_fd);
        close(server_fd);
        return -1;
    }
    
    // Wait for client to finish processing
    sleep(1);
    
    // Cleanup
    close(client_fd);
    close(server_fd);
    
    return 0;
}

// Client function
static int run_client(void) {
    int client_fd;
    struct sockaddr_un server_addr;
    char buffer[BUFFER_SIZE];
    ssize_t bytes;
    
    // Wait for server to be ready
    sleep(1);
    
    printf("[Client] Creating PF_UNIX SOCK_STREAM socket...\n");
    client_fd = socket(PF_UNIX, SOCK_STREAM, 0);
    if (client_fd < 0) {
        perror("Failed to create client socket");
        return -1;
    }
    
    // Set up server address
    memset(&server_addr, 0, sizeof(server_addr));
    server_addr.sun_family = AF_UNIX;
    strncpy(server_addr.sun_path, SOCKET_PATH, sizeof(server_addr.sun_path) - 1);
    
    printf("[Client] Connecting to server path: %s\n", SOCKET_PATH);
    if (connect(client_fd, (struct sockaddr *)&server_addr, sizeof(server_addr)) < 0) {
        perror("Failed to connect to server");
        close(client_fd);
        return -1;
    }
    printf("[Client] Connected to server\n");
    
    // Send message to server
    const char *message = "Hello server, I am the client!";
    printf("[Client] Sending message: %s\n", message);
    if (send(client_fd, message, strlen(message), 0) < 0) {
        perror("Failed to send message");
        close(client_fd);
        return -1;
    }
    
    // Receive response from server
    memset(buffer, 0, BUFFER_SIZE);
    bytes = recv(client_fd, buffer, BUFFER_SIZE - 1, 0);
    if (bytes < 0) {
        perror("Failed to receive response");
        close(client_fd);
        return -1;
    }
    printf("[Client] Received response from server: %s\n", buffer);
    
    // Cleanup
    close(client_fd);
    
    return 0;
}

int main(int argc, char *argv[]) {
    pid_t pid;
    
    printf("=== PF_UNIX SOCK_STREAM Socket Communication Example ===\n");
    
    // Create child process
    pid = fork();
    if (pid < 0) {
        perror("Failed to create child process");
        return EXIT_FAILURE;
    }
    
    if (pid == 0) {
        // Child process: client
        printf("\n--- Client Process (PID: %d) ---\n", getpid());
        if (run_client() < 0) {
            printf("Client execution failed\n");
        }
        printf("Client process ended\n");
        exit(0);
    } else {
        // Parent process: server
        printf("\n--- Server Process (PID: %d) ---\n", getpid());
        if (run_server() < 0) {
            printf("Server execution failed\n");
        }
        
        // Wait for child process to finish
        waitpid(pid, NULL, 0);
        
        // Cleanup socket file
        cleanup_socket(SOCKET_PATH);
        
        printf("\nServer process ended\n");
        printf("=== Program execution completed ===\n");
    }
    
    return EXIT_SUCCESS;
}

NuttX log:

NuttShell (NSH) NuttX-12.12.0-RC0
MOTD: username=admin password=Administrator
nsh> hello
=== PF_UNIX SOCK_STREAM Socket Communication Example ===

--- Server Process (PID: 4) ---
[Server] Creating PF_UNIX SOCK_STREAM socket...
[Server] Binding to path: local_socket
[Server] Starting to listen for connections...
[Server] Waiting for client connection...

--- Client Process (PID: 5) ---
[Client] Creating PF_UNIX SOCK_STREAM socket...
[Client] Connecting to server path: local_socket
[Client] Connected to server
[Client] Sending message: Hello server, I am the client!
[Server] Client connected
[Server] Received message from client: Hello server, I am the client!
[Server] Sending response: Hello client, I am the server!
[Client] Received response from server: Hello client, I am the server!
Client process ended

Server process ended
=== Program execution completed ===
nsh> 

@github-actions github-actions bot added Area: Networking Effects networking subsystem Size: M The size of the change in this PR is medium labels Dec 30, 2025
anchao
anchao previously requested changes Dec 30, 2025
Copy link
Contributor

@anchao anchao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not reuse net_lock and extended it to support lock instances? if in this way, there's no need to add unlock/lock operations in each branch, which can avoid lock imbalance issues and eliminate the need to worry about whether to call lock or unlock at the code level.

@zhhyu7
Copy link
Contributor Author

zhhyu7 commented Dec 30, 2025

Why not reuse net_lock and extended it to support lock instances? if in this way, there's no need to add unlock/lock operations in each branch, which can avoid lock imbalance issues and eliminate the need to worry about whether to call lock or unlock at the code level.

@anchao If we directly modify the net_lock prototype, too much code needs to be changed, moreover, it's possible that some net_lock in external repository drivers may be affected, leading to compilation errors. Therefore, each relatively independent protocol has added its own global lock. I will submit a patch for code format optimization later, changing all device-related protocol code to a format similar to the previous net_xxx_wait. For local socket, I can also add a unified interface with parameters and breaklock to optimize the code in the unlock/wait/lock format. Do you think this is reasonable?
https://github.com/apache/nuttx/pull/17734/files#diff-85924974029d3e344dd6da506275a7cc78cde793d529d8336bed944144beda55R494

@xiaoxiang781216
Copy link
Contributor

Why not reuse net_lock and extended it to support lock instances? if in this way, there's no need to add unlock/lock operations in each branch, which can avoid lock imbalance issues and eliminate the need to worry about whether to call lock or unlock at the code level.

net_lock/net_unlock need keep the prototype without change since many net driver call net_lock/net_unlock and we don't have any plan to change them.

@anchao
Copy link
Contributor

anchao commented Dec 31, 2025

@xiaoxiang781216 @zhhyu7
Alternatively, we can add a dedicated set of APIs for the new local locks while keeping the prototype of net_lock unchanged.

extension net/utils/net_lock.c -> net/utils/net_lock_ext.c

net_lock/net_unlock need keep the prototype without change since many net driver call net_lock/net_unlock and we don't have any plan to change them.

I have another question, if net_lock in the board code is not properly adjusted, wouldn't this lead to deadlocks or race conditions? This is because after the modification, there will be 2 separate locks (one for the protocol stack and one for the board), and I am concerned that some callback strategies may result in ABBA dead lock

@zhhyu7
Copy link
Contributor Author

zhhyu7 commented Dec 31, 2025

@xiaoxiang781216 @zhhyu7 Alternatively, we can add a dedicated set of APIs for the new local locks while keeping the prototype of net_lock unchanged.

extension net/utils/net_lock.c -> net/utils/net_lock_ext.c

net_lock/net_unlock need keep the prototype without change since many net driver call net_lock/net_unlock and we don't have any plan to change them.

I have another question, if net_lock in the board code is not properly adjusted, wouldn't this lead to deadlocks or race conditions? This is because after the modification, there will be 2 separate locks (one for the protocol stack and one for the board), and I am concerned that some callback strategies may result in ABBA dead lock

@anchao Yes, If there is a callback strategy that may lead to ABBA deadlock, the corresponding driver will need to make relevant adaptations. Currently, we can only ensure that processes that net_lock is not first attempted to be acquired in the callback is compatible.

@xiaoxiang781216 xiaoxiang781216 requested a review from anchao January 4, 2026 04:11
@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 @zhhyu7 Alternatively, we can add a dedicated set of APIs for the new local locks while keeping the prototype of net_lock unchanged.

extension net/utils/net_lock.c -> net/utils/net_lock_ext.c

net_lock/net_unlock need keep the prototype without change since many net driver call net_lock/net_unlock and we don't have any plan to change them.

I have another question, if net_lock in the board code is not properly adjusted, wouldn't this lead to deadlocks or race conditions? This is because after the modification, there will be 2 separate locks (one for the protocol stack and one for the board), and I am concerned that some callback strategies may result in ABBA dead lock

Most driver code hold net lock only in the work thread, so it isn't a problem.

remove the use of net_lock in the local module and decouple it from
other network modules.

Signed-off-by: zhanghongyu <[email protected]>
modify the code of the adapted protocol stack to avoid deadlocks and the
logic that cannot be protected by locks after modification.

Signed-off-by: zhanghongyu <[email protected]>
@Donny9 Donny9 merged commit fa6c857 into apache:master Jan 6, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: simulator Issues related to the SIMulator Area: Drivers Drivers issues Area: Networking Effects networking subsystem Area: USB Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants