40 ms·

Holy smokes: https://github.com/jkfran/killport/blob/main/src/linux.rs#L84 https://github.com/jkfran/killport/blob/main/src/linux.rs#L8...

• Deeg9rie9usi 3y ago
  Hah, I was about to rant about the same. This tool is obviously not battle proved and never run outside the developer's machine.
  • skrebbel 3y ago
    care to enlighten us mere mortals? why is this a holy smokes?
    • TheDong 3y ago
      A loop over all fds across all processes on a box will be very slow on any large machine with, say, a few hundred thousand processes, some of which may have up to hundreds of thousands or millions of FDs open. Especially since it looks like it's reading the owning process's cmdline per-fd.
      • Aissen 3y ago
        AFAIK there's no other ways to do this on Linux: link a given tcp connection to a process. The network uAPIs (netlink or /proc/net/tcp?) will give you an inode, and to link the inode to the PID you need to go through every open fd in /proc/*/fd. ss, fuser or lsof (mentioned in other comments) do this too.
        • TheDong 3y ago
          Yup, ss and friends do this, but they don't read "/proc/$pid/cmdline" N times (where N is the number of file descriptors the process has) in the hotloop. I phrased it poorly. Doing the loop is what it is, but doing the loop and allocating inside (which 'process.cmdline()' does) on every loop is something I'm fairly sure none of the other tools do.
          • Aissen 3y ago
            Oh yeah, definitely. In my implementation, even skipping reading comm (once!) if not needed gave me better perf than ss: https://github.com/anisse/tcpkill/blob/cfd96d5dec438a3722edb25938a88aa7671fae84/src/main.rs#L72-L82 https://github.com/anisse/tcpkill/blob/cfd96d5dec438a3722edb...
          • scottlamb 3y ago
            > Yup, ss and friends do this, but they don't read "/proc/$pid/cmdline" N times (where N is the number of file descriptors the process has) in the hotloop. This one doesn't either. The code structure could be clearer IMHO, but `kill_processes_by_inode` reads the cmdline within the `if target_inode == inode` block, which breaks out of the `for fd in fds` loop at the end. So it only looks at the cmdline once per process that has the target inode. That said, if `find_target_inodes` returns n inodes, `kill_processes_by_port` will call `kill_processes_by_inode` n times. It'd be better to find all fds only once and compare each to all the target inodes at once with a hash set (if n might be large) or by bisecting a sorted slice. Multiple inodes per port could happen in a few different ways: different processes listening to the same port on different IPv4/IPv6 addresses, an old-fashioned pre-forked sort of server model, a bunch of individually spawned single-threaded servers listening on the same port via `SO_REUSEPORT`/`SO_REUSEADDR`.
      • loeg 3y ago
        > A loop over all fds across all processes on a box will be very slow on any large machine with, say, a few hundred thousand processes, some of which may have up to hundreds of thousands or millions of FDs open. Any implementation of this objective has the same limitation, though.
      • deleted 3y ago
        [deleted]
    • Deeg9rie9usi 3y ago
      The overly usage of unwrap() is also suspicious. Processes can vanish while you are inspecting them. So there is a good chance that this tool will panic on busy machines.
• VWWHFSfQ 3y ago
  My other holy smokes moment was the install sh script hosted on bitly > curl -sL https://bit.ly/killport https://bit.ly/killport | sh
  • panki27 3y ago
    A shame it doesn't require "sudo sh" to install.