The tool said it wrote 48 packets. Only 44 were there

Recap

I have been developing a tool wrapped around tshark. The first blog post was about hitting a wall on a 2.5 GB file. Later, I talked about parallelizing the PCAP processing in my second blog post where I ran into a file corruption bug.

Now here comes the real culprit: the PcapSplitter's FiveTupleSplitter caused file truncation/corruption on TCP session reuse (i.e., a new SYN packet arrives for an already tracked 5-tuple hash).

How I found it

Post two ended with me replacing PcapSplitter. This is what came before that replacement.

The initial signal came from "Total Block Length" errors thrown by tshark on some output files. That told me something was wrong, not what.

The actual signal came when I built a small reproduction. PcapSplitter reported 12 files and 48 packets, but on disk, there were 11 files and 44 packets. Exit code zero and printed "Finished" on standard output.

Theory One: File Descriptor Exhaustion, wrong

Someone on Reddit suggested file descriptor exhaustion. It was plausible with one output file per flow, where I had 95-125 flows, and a failed open() with an unchecked return would produce silent drops.

But instead of relying on theory, I tested it. Then I found that at low ulimit -n it silently drops most packets and still exits zero. Sharp threshold, reproducible. It wasn't my corruption. Raising the limit didn't fix the original problem.

Theory Two: Hardcoded LRU Limit, again wrong

After testing file descriptor exhaustion, I found that the PcapSplitter library has a hardcoded MAX_NUMBER_OF_CONCURRENT_OPEN_FILES = 250 with an LRU that closes and reopens handles past it. It was a perfect candidate.

I tested it too. It held up fine. My minimal reproduction was 13 connections, nowhere near the cap.

Two good theories, one found a different real bug, the other held up fine. Neither was mine.

What it was exactly

The correct part of the splitter was assigning a new file number when a TCP session reuses a 5-tuple, but the filename function builds the name from IP and port only. So, in this case, both sessions get the same filenames. Then main.cpp sees a file number it has never seen, and opens that file fresh, without append. This truncates the existing file or causes a race condition between two active file writer handlers.

1. File Number Allocation on Session Reuse (ConnectionSplitters.h)

When a fresh SYN arrives for a 5-tuple hash that has been seen before, FiveTupleSplitter::getFileNumber deliberately assigns a new file number to isolate the logical session:

if (isSyn && m_TcpFlowTable.find(hash) != m_TcpFlowTable.end() && m_TcpFlowTable[hash] == false)
{
    m_FlowTable[hash] = getNextFileNumber(filesToClose);   // <-- Allocates a NEW file number
}

2. The Filename Disconnect (ConnectionSplitters.h)

Right below this logic, FiveTupleSplitter::getFileName overrides the base class implementation but completely ignores the fileNumber parameter passed into it:

std::string getFileName(pcpp::Packet& packet, const std::string& outputPcapBasePath, int fileNumber)
{
    // ...
    sstream << "connection-";
    if (packet.isPacketOfType(pcpp::TCP)) {
        // ...
        // Filename is derived strictly from the packet's IP/Port values:
        updateStringStream(sstream, getSrcIPString(packet), srcPort, getDstIPString(packet), dstPort);
        return outputPcapBasePath + sstream.str();   // <-- fileNumber parameter is UNUSED on the TCP/UDP path
    }
    // ...
}

Because fileNumber is dropped, the new session produces the identical filename string as the older session.

3. The Truncation / Race Condition (main.cpp)

In main.cpp, the active writer cache map (outputFiles) is keyed by the integer fileNum, not the filepath string. When the newly allocated fileNum is evaluated, it goes down the fresh initialization branch:

// Since fileNum is a newly generated integer, it won't be found in the map
if (outputFiles.find(fileNum) == outputFiles.end()) 
{
    std::string fileName = splitter->getFileName(parsedPacket, outputPcapFileName, fileNum) + outputFileExtension;

    if (isReaderPcapng) {
        outputFiles[fileNum].reset(new pcpp::PcapNgFileWriterDevice(fileName));
    } else {
        outputFiles[fileNum].reset(new pcpp::PcapFileWriterDevice(fileName, rawPacket.getLinkLayerType()));
    }

    // CRITICAL: Plain open — no append flag — on a path that already holds valid session data!
    if (!outputFiles[fileNum]->open()) 
        break;
}

Impact & Consequences

1. File Truncation: If the previous file representing the same session was closed by the LRU mechanism, the new file writer triggers a standard file open on the existing path, that truncates the existing PCAP data captured from the previous session.

2. Write Race / Corruption: If duplicate files representing the same session are still open and active, it creates a race condition between two active file writer handlers. This breaks the linear layout of the PCAP/PCAPNG format and leaves the file corrupted.

Reporting the bug and fixing upstream

I filed the issue with a 13-connection reproduction in #2248.

The maintainer found his own commit 6379100 and said he couldn't remember why he'd made the change. He pushed back on my first fix, where I suggested appending the number to every filename. But it changes output for everyone, which is why he pushed back. Fair point.

Then I proposed a middle ground where we only suffix on actual collision, first session keeps its name. He accepted it, asked for tests. I wrote the code and tests.

Merged #2249

Takeaways

1. Exit code zero is not always success. Verify what a tool claims against what it produced.

2. A possible theory that reproduces a real bug can still be the wrong theory.

3. Bugs usually come from two reasonable components disagreeing about a definition. Not necessarily a broken piece.