Skip to content

FileCredentialRevocationStore reopens the journal file on every append, risking durability loss on crash #56

@nficano

Description

@nficano

In arcp-runtime/src/main/java/dev/arcp/runtime/credentials/FileCredentialRevocationStore.java the append method around line 85 opens a fresh BufferedWriter via Files.newBufferedWriter(path, ..., StandardOpenOption.CREATE, StandardOpenOption.APPEND) for every event written, with a try-with-resources that closes (and therefore flushes) immediately. This has two problems. First, file open and close are expensive — a credential revocation store that records every issue and revoke event will pay disk-level syscalls per credential. Second and more importantly, BufferedWriter.close() flushes user-space buffers down to the OS but does not fsync. On power loss or kernel crash between the close and the OS flushing its page cache, the most recent revocation records can be lost. For a store whose entire job is durable bookkeeping of "which credentials are still live," loss after acknowledgment is a real correctness failure: the runtime believes a credential was revoked when the upstream provider still considers it valid (or the inverse — the store believes it is outstanding when the provider has released it).\n\nFix prompt: In arcp-runtime/src/main/java/dev/arcp/runtime/credentials/FileCredentialRevocationStore.java keep a long-lived FileChannel (or a long-lived BufferedWriter wrapping a FileOutputStream whose FileDescriptor you can sync) as a private field, opened once at construction and closed via a new public close() method that callers invoke at shutdown. In the append method, write the line through that persistent writer and follow each write with channel.force(false) (or fos.getFD().sync()) before returning, so the caller observes durable acknowledgment. Add an implements AutoCloseable to the class and document the close contract. The synchronized methods already serialize file access, so single-threaded fsync is straightforward. Add a JUnit test that writes 100 events, retrieves them via outstanding(), then constructs a fresh store over the same path and asserts the recovered outstanding list matches what the first store believed it had recorded.

Metadata

Metadata

Assignees

No one assigned

    Labels

    performancePerformance and resource-efficiency workseverity:mediumMedium severity

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions