Skip to content

TestFilesCreateCLICancelClosesStalledFIFO is red on macOS: Close() cannot release a FIFO read blocked in the kernel #209

Description

@feiiiiii5

What I see

pkg/cmd/multipartresource_unix_test.go builds for darwin (//go:build !windows), and on macOS it fails deterministically:

$ go test ./pkg/cmd -run TestFilesCreateCLICancelClosesStalledFIFO -count=2
--- FAIL: TestFilesCreateCLICancelClosesStalledFIFO (0.00s)
    multipartresource_unix_test.go:129:
        	Error Trace:	.../pkg/cmd/multipartresource_unix_test.go:129
        	Error:      	Expected error with "broken pipe" in chain but got nil.
        	Test:       	TestFilesCreateCLICancelClosesStalledFIFO
        	Messages:   	cancellation must close the owned FIFO reader
FAIL	github.com/openai/openai-cli/pkg/cmd	0.521s

Everything before line 129 passes, including require.ErrorIs(err, context.Canceled): the request is canceled promptly. Only "the producer must observe EPIPE" fails.

The CLI does reach its own cleanup

I temporarily instrumented the two cleanup sites (reverted, working tree clean) and the whole chain runs:

TEMP-DEBUG Close called                  # multipartRequestBody.Close (pkg/cmd/multipartbody.go:84)
TEMP-DEBUG fileUpload.Close false        # *os.File.Close() returned nil (pkg/cmd/multipartfile.go:27)
TEMP-DEBUG cleanup done false            # closeFileUploads succeeded (pkg/cmd/multipartbody.go:122)

So Close() is called on the FIFO's read end and reports success, and the producer's next Write still returns nil.

Standalone reproduction

Same shape without the CLI: open a FIFO read end with os.Open, drain what the producer wrote, park a second Read, then Close().

package main

import (
	"fmt"
	"os"
	"path/filepath"
	"syscall"
	"time"
)

func main() {
	dir, _ := os.MkdirTemp("", "fifo")
	path := filepath.Join(dir, "p.fifo")
	if err := syscall.Mkfifo(path, 0o600); err != nil {
		fmt.Println("mkfifo:", err)
		return
	}

	probe := make(chan chan<- error, 1)
	go func() { // the upload producer
		w, err := os.OpenFile(path, os.O_WRONLY, 0)
		if err != nil {
			fmt.Println("producer open:", err)
			return
		}
		defer w.Close()
		if _, err := w.Write([]byte("partial upload")); err != nil {
			fmt.Println("producer first write:", err)
			return
		}
		result := <-probe
		_, err = w.Write([]byte("still open"))
		result <- err
	}()

	f, err := os.Open(path) // what openFileUpload does for a non-regular file
	if err != nil {
		fmt.Println("open:", err)
		return
	}
	buf := make([]byte, 14)
	if n, err := f.Read(buf); n != 14 || err != nil {
		fmt.Println("drain read:", n, err)
		return
	}

	blocked := make(chan string, 1)
	go func() { // the encoder goroutine
		n, err := f.Read(buf)
		blocked <- fmt.Sprintf("returned n=%d err=%v", n, err)
	}()
	time.Sleep(300 * time.Millisecond)

	fmt.Println("deadline support:", f.SetReadDeadline(time.Now()))
	fmt.Println("Close:", f.Close())
	select {
	case msg := <-blocked:
		fmt.Println("blocked read", msg)
	case <-time.After(2 * time.Second):
		fmt.Println("blocked read is still blocked 2s after Close")
	}

	pr := make(chan error, 1)
	probe <- pr
	select {
	case err := <-pr:
		fmt.Println("producer write after Close:", err)
	case <-time.After(2 * time.Second):
		fmt.Println("producer write blocked")
	}
}
$ go run .
deadline support: file type does not support deadline
Close: <nil>
blocked read is still blocked 2s after Close
producer write after Close: <nil>

Why

From openFileNolog in Go 1.25's src/os/file_unix.go:

	// Things like regular files and FIFOs in kqueue on *BSD/Darwin
	// may not work properly (or accurately according to its manual).
	// As a result, we should avoid adding those to the kqueue-based
	// netpoller. Check out #19093, #24164, and #66239 for more contexts.
	...
			// In addition to the behavior described above for regular files,
			// on Darwin, kqueue does not work properly with fifos:
			// closing the last writer does not cause a kqueue event
			// for any readers. See issue #24164.
			if (runtime.GOOS == "darwin" || runtime.GOOS == "ios") && typ == syscall.S_IFIFO {
				pollable = false
			}

A darwin FIFO read end is therefore outside the runtime poller (measured above: SetReadDeadline returns file type does not support deadline), so Read is a raw blocking syscall. Close() drops our descriptor-table entry, but the goroutine still parked in read keeps the FIFO's reader count alive — the producer is never told to stop, and the encoder goroutine never returns. openFileUpload takes precisely this shape for non-regular files: an unknown-size upload keeps a bare *os.File (pkg/cmd/multipartfile.go:124-129).

The demotion is darwin/ios-only, so I expect the assertion to hold on Linux, but I could not verify that here: no Linux runner was available, and every job in .github/workflows is runs-on: ubuntu-latest, so no CI job exercises this file on darwin.

Impact

On macOS, canceling a streaming FIFO upload (openai files create --file ./p.fifo plus Ctrl-C, or any context cancellation) leaves the encoder goroutine parked in read for the lifetime of the process and leaves an external producer blocked in write instead of getting EPIPE. For a one-shot CLI invocation the process exits shortly after, so the visible symptom today is mainly that the invariant this test encodes does not hold on darwin; it matters more for the importable pkg/cmd path, where the leak persists.

Options I can see

  1. Give unknown-size uploads a cancellable reader that owns its own wait — put the descriptor in non-blocking mode and loop over poll/kqueue with a bounded slice, checking a closed flag and the context between slices. Deterministic on both platforms, and it never depends on the kernel waking a blocked read, at the cost of new unix-only code in the upload path plus tests for the EOF and partial-read boundaries.
  2. Keep the current design and narrow the claim: assert the EPIPE release only where the platform can provide it, and on darwin assert what actually happens (request canceled, cleanup called) instead, with the stdlib reference next to the skip so the difference is documented rather than hidden.
  3. Add a darwin test job (at least for pkg/cmd) so a platform-dependent cancellation contract cannot silently rot. Note scripts/test also compiles GOOS=windows go test -c ./..., so a darwin-only assumption is currently invisible in both directions.

One route I looked for and ruled out: forcing the FIFO into the poller by opening the descriptor with syscall.Open and handing it to os.NewFile (which skips the demotion because kind != kindOpenFile). Go's stated reason for the demotion is that a kqueue FIFO reader misses the "last writer closed" event, so this risks stalling at the end of a successful upload — trading a cancel-time leak for a happy-path hang.

Environment

  • go version go1.25.0 darwin/arm64, macOS 27.2
  • openai-cli at 0169bff050ff9c05c4609b669c06709306d464fa (chore(deps): update openai-go to v3.64.0 (#206)), working tree clean
  • No credentials, live requests, or customer data involved: only httptest and paths under t.TempDir() / os.MkdirTemp

AI-assisted: an AI coding agent ran every command above and read its output on the account owner's standing instruction for this repo. No human reviewed this report before it was posted. The instrumentation lines quoted above were temporary and are not in the tree.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions