Commit ec8e43b8 authored by Ramon Nou's avatar Ramon Nou
Browse files

fix: preserve stdio fds and malleability host files 

Avoid closing kernel stdin, stdout, or stderr when removing GKFS virtual
file descriptor mappings, preventing libc or logging descriptors from
being invalidated during teardown.

Update integration test forwarding daemon setup to preserve host files for
malleability tests and simplify gkfs_malleability invocation paths.
parent 012106da

src/client/open_file_map.cpp

+10 −0
Original line number Diff line number Diff line
@@ -260,6 +260,16 @@ OpenFileMap::remove(const int fd) { 
    shard.files.erase(fd);

    total_files_--;



    if(fd >= 0 && fd < 3) {

        // A GKFS file descriptor may intentionally be mapped onto stdin,

        // stdout, or stderr via dup2()/dup3() (GNU dd does this for its input

        // and output files). In that case the numeric descriptor can also name

        // a real kernel stdio fd. The GKFS close path must only remove the

        // virtual mapping; closing the kernel fd here may invalidate

        // libc/logging descriptors during process teardown.

        return true;

    }



    if(!CTX->protect_fds()) {

        if(!CTX->range_fd()) {

            // We close the dev null fd

tests/integration/conftest.py

+3 −1
Original line number Diff line number Diff line
@@ -215,7 +215,9 @@ def gkfwd_daemon_factory(test_workspace, request): 


    interface = request.config.getoption('--interface')



    return FwdDaemonCreator(interface, test_workspace)

    keep_hosts = "malleability" in request.node.nodeid.split("/")



    return FwdDaemonCreator(interface, test_workspace, keep_hosts=keep_hosts)



@pytest.fixture

def gkfwd_client_factory(test_workspace):

tests/integration/conftest.template

+3 −1
Original line number Diff line number Diff line
@@ -215,7 +215,9 @@ def gkfwd_daemon_factory(test_workspace, request): 


    interface = request.config.getoption('--interface')



    return FwdDaemonCreator(interface, test_workspace)

    keep_hosts = "malleability" in request.node.nodeid.split("/")



    return FwdDaemonCreator(interface, test_workspace, keep_hosts=keep_hosts)



@pytest.fixture

def gkfwd_client_factory(test_workspace):

tests/integration/harness/gkfs.py

+12 −5
Original line number Diff line number Diff line
@@ -239,11 +239,12 @@ class FwdDaemonCreator: 
    Factory that allows tests to create forwarding daemons in a workspace.

    """



    def __init__(self, interface, workspace):

    def __init__(self, interface, workspace, keep_hosts=False):

        self._interface = interface

        self._workspace = workspace

        self._keep_hosts = keep_hosts



    def create(self, expand_mode=False, enable_forwarding=True):

    def create(self, expand_mode=False, enable_forwarding=True, keep_hosts=None):

        """

        Create a forwarding daemon in the tests workspace.
@@ -257,10 +258,14 @@ class FwdDaemonCreator: 
        The `FwdDaemon` object to interact with the daemon.

        """



        if keep_hosts is None:

            keep_hosts = self._keep_hosts



        daemon = FwdDaemon(self._interface,

                           self._workspace,

                           expand_mode=expand_mode,

                           enable_forwarding=enable_forwarding)

                           enable_forwarding=enable_forwarding,

                           keep_hosts=keep_hosts)

        daemon.run()



        return daemon
@@ -1402,7 +1407,7 @@ class ShellClientLibc: 
        return self._workspace.twd



class FwdDaemon:

    def __init__(self, interface, workspace, expand_mode=False, enable_forwarding=True):

    def __init__(self, interface, workspace, expand_mode=False, enable_forwarding=True, keep_hosts=False):



        self._address = get_ephemeral_address(interface)

        self._workspace = workspace
@@ -1411,6 +1416,7 @@ class FwdDaemon: 
        self._env = os.environ.copy()

        self._expand_mode = expand_mode

        self._enable_forwarding = enable_forwarding

        self._keep_hosts = keep_hosts

        self._rootdir = self._workspace.rootdir

        self._metadir = self._workspace.metadir

        self._logdir = self._workspace.logdir
@@ -1432,8 +1438,9 @@ class FwdDaemon: 
            'GKFS_HOSTS_FILE'      : str(self.cwd / gkfwd_hosts_file),

            'GKFS_DAEMON_LOG_PATH' : str(self.logdir / gkfwd_daemon_log_file),

            'GKFS_DAEMON_LOG_LEVEL': gkfwd_daemon_log_level,

            'GKFS_DAEMON_KEEP_HOSTS_FILE': 'ON'

        }

        if keep_hosts:

            self._patched_env['GKFS_DAEMON_KEEP_HOSTS_FILE'] = 'ON'

        if expand_mode:

            self._patched_env['GKFS_DAEMON_EXPAND'] = 'ON'

tests/integration/malleability/test_client_disconnect_during_rpc.py

+3 −24
Original line number Diff line number Diff line
@@ -37,7 +37,6 @@ which contains NA_NOENTRY errors when clients vanish during RPC handling. 
import os

import subprocess

import time

import shutil

from pathlib import Path

import pytest
@@ -48,22 +47,16 @@ def test_daemon_survives_client_abort_during_write(gkfwd_daemon_factory, gkfs_sh 
    time.sleep(5)



    hostfile = Path(d00.hostfile)

    search_path = ":".join(str(p) for p in gkfs_shell._search_paths)

    malleability_bin = shutil.which("gkfs_malleability", path=search_path)

    assert malleability_bin is not None, "gkfs_malleability not found in PATH"



    # Write some data first to establish the file exists

    libdirs = gkfs_shell._patched_env.get("LD_LIBRARY_PATH", "")

    ret = gkfs_shell.bash(

        f"LD_LIBRARY_PATH={libdirs} LIBGKFS_HOSTS_FILE={hostfile} "

        f"LIBGKFS_HOSTS_FILE={hostfile} "

        f"dd if=/dev/zero of={d00.mountdir}/test_abort_file bs=1024 count=1 2>&1"

    )



    # Verify daemon works before abort

    cmd_str = (

        f"LD_LIBRARY_PATH={libdirs} "

        f"LIBGKFS_HOSTS_FILE={hostfile} "

        f"{malleability_bin} mutate status"

        f"LIBGKFS_HOSTS_FILE={hostfile} gkfs_malleability mutate status"

    )

    cmd = gkfs_shell.script(cmd_str, intercept_shell=False)

    assert cmd.exit_code == 0, f"Daemon not responding before client abort: {cmd.stderr.decode()}"
@@ -82,13 +75,6 @@ def test_safe_respond_handles_vanished_client(gkfwd_daemon_factory, gkfs_shell): 


    hostfile = Path(d00.hostfile)



    # Verify safe_respond wrapper exists in the compiled source

    search_path = ":".join(str(p) for p in gkfs_shell._search_paths)

    daemon_bin = shutil.which("gkfs_daemon", path=search_path)

    assert daemon_bin is not None, "gkfs_daemon not found in PATH"



    # Check objdump for safe/respond symbols

    ret = gkfs_shell.bash(f"objdump -t {daemon_bin} 2>/dev/null | grep -i safe | wc -l || echo 0")

    # The test just needs to pass if the daemon is functional

    # Verify the daemon survives shutdown and restart

    d00.shutdown()
@@ -110,7 +96,6 @@ def test_multiple_rapid_client_disconnections(gkfwd_daemon_factory, gkfs_shell): 
    time.sleep(5)



    hostfile = Path(d00.hostfile)

    libdirs = gkfs_shell._patched_env.get("LD_LIBRARY_PATH", "")



    # Simulate multiple rapid disconnections

    for i in range(5):
@@ -118,7 +103,6 @@ def test_multiple_rapid_client_disconnections(gkfwd_daemon_factory, gkfs_shell): 
        client_proc = subprocess.Popen(

            ["python3", "-c", f"""

import os, time

os.environ['LD_LIBRARY_PATH'] = '{libdirs}'

os.environ['LIBGKFS_HOSTS_FILE'] = '{hostfile}'

fd = os.open('{d00.mountdir}/test_rapid_{i}', os.O_CREAT | os.O_WRONLY, 0o644)

if fd >= 0:
@@ -133,14 +117,9 @@ if fd >= 0: 
        time.sleep(0.5)



    # Verify daemon survived all disconnections

    search_path = ":".join(str(p) for p in gkfs_shell._search_paths)

    malleability_bin = shutil.which("gkfs_malleability", path=search_path)

    assert malleability_bin is not None, "gkfs_malleability not found in PATH"



    cmd_str = (

        f"LD_LIBRARY_PATH={libdirs} "

        f"LIBGKFS_HOSTS_FILE={hostfile} "

        f"{malleability_bin} mutate status"

        f"LIBGKFS_HOSTS_FILE={hostfile} gkfs_malleability mutate status"

    )

    cmd = gkfs_shell.script(cmd_str, intercept_shell=False)

    assert cmd.exit_code == 0, \