Devlog: TODOs and Bugs

[Direwolfesp]

This commit 1845112 is full of errors and its just a mock-up on how can I architecture this, but it needs lots of proper testing. Some todos and bugs are:

• IOMessage does not own the payload and becomes invalid. Possible solutions are: IOMessage owns the payload (simple but less performant), Transfer ownership to the IOMessage and the receiver of the message frees it.
• IOMessage does not relates a given message with the original sender. As stated in my FIXME so we can pass a pointer to a Client and a callback so he can update its state.
• related: manage piece buffer (here)
• while (true) {connect()} is illegal in non-blocking mode. I has to return WouldBlock and wait for EPOLLOUT (here)
• Reader and writer state machines assume async handling: i dont detect partial writes, and dont flush buffered writes correctly (here)
• Maybe separate in handleReadNormal handleWriteNormal, also somes typos: (here)
• filesystem busy waits wastes time (here)
• Some Message validation: HAVE could have nonexistent pieces, PIECE blocks duplicated, unexpected messages.

[Direwolfesp]

from commit b20cafb. I tried sending the bitfield first and wait for him to respond with his bitfield, however it crashes with out of bound index when trying to register his peers. getNumPieces() is returning bad numbers, why¿?¿?¿?¿'

• fix assert (here) edit: It was just some dangling pointer to stack allocated object.

[Direwolfesp]

Currently:

• Filesystem thread: in charge of managing the mmaped files. It uses two spsc queues to receive IOMessages, a message contain the piece buffer that needs to be hashed and stored, and the client who requested that message. Once it processes that task it enqueues back the result (success/fail), then the main loop can notify each client that the task it pushed earlier is either complete or failed. Problems: This enforces the rule that a piece must correspond to only one peer, which lacks flexibility for a future end-game mode for example and adds more complexity to the global download state, each piece buffer is isolated within a PeerConnection and other PeerConnections cannot cooperate in the same piece which could be needed at some point.
• Main thread: has a bunch of PeerConnections and a piece picker. The piece picker picks the rarest piece a peer has and marks blocks as requested or finished, however it does not contain any block picking logic nor any block status info apart from the status. Also, the picker wont choose a piece an other peer is downloading. Due to that, each PeerConnection picks all blocks from his selected piece in an isolated way.

Proposal

• Filesystem thread: we take advantage that we have all the files mmaped to memory to not duplicate piece buffers and to not pass them around and just expose an API for writingBlocks itself, so whenever a PeerConnection downloads a block it instantaneously sends it to the filesystem object (that function does not run in other thread and should not be part of the critical section), so the only messages that will be sent to the thread and run in parallel are going to be the hashing, which can be also done in-place (just readying the mmaped file, without passing more memory). The kernel buffer those writes in its cache so I dont think it will block because its not really IO. Also, the filesystem thread is the one that has final word about if the torrent download is complete or not. How do we notify back that result? Maybe we can enqueue a IOMessage of download_finished and the main loop can just shut down cleanly.
• Main thread: now, each PeerConnection should not has its own piece buffer but just ask for the picker what block download next and send it back to Filesytem object synchronously without blocking, and if the PeerConnection has received all bytes for the piece, enqueue the task of hashing the piece. In PeerConnection maybe tidy up the labor of managing the download (request pipeline, bandwith, downloaded,...) in other struct. The PiecePicker should incorporate more block logic other than just the state (index, offset, length, etc..) and expose an API to pickBlock
• Some concerns: ensure block writes to multiple files correctly. Also when hashing a piece, the main loop should not write to the same piece.

[Direwolfesp]

Fix:

main is triggering markPieceCompleted when the piece blocks are not marked as downloaded yet

[DEBUG] peer: [165.232.35.114:51443] Entering handle read
[DEBUG] peer: [165.232.35.114:51443] peer sent piece: { .index = 2, .begin = 16384}
[DEBUG] PiecePicker: updated block 1 from piece 2 to downloaded
[INFO] filesystem: got a new submission: action = check_integrity, piece = 2
[DEBUG] peer: [165.232.35.114:51443] Entering handle write
[WARNING] peer: [165.232.35.114:51443] request: could not pick block
[INFO] filesystem: Processing task with id: check_integrity
[DEBUG] filesystem: Piece #2 verified successfully
[INFO] manager: Processing disk_io message: piece_completed
thread 534420 panic: A piece that didn't finished downloading was marked as completed
/home/dire/Documents/Coding/github/zigtorrent/src/PiecePicker.zig:204:17: 0x1205332 in markPieceCompleted (main.zig)
                @panic("A piece that didn't finished downloading was marked as completed");
                ^
/home/dire/Documents/Coding/github/zigtorrent/src/manager.zig:244:47: 0x11c7c8a in onIOMessage (main.zig)
                self.picker.markPieceCompleted(io_message.index);
                                              ^
/home/dire/Documents/Coding/github/zigtorrent/src/manager.zig:228:33: 0x11ab248 in run (main.zig)
                self.onIOMessage(io_msg);
                                ^
/home/dire/Documents/Coding/github/zigtorrent/src/main.zig:33:20: 0x11abac1 in main (main.zig)
    try session.run();
                   ^
/usr/lib/zig/std/start.zig:627:37: 0x11ac689 in posixCallMainAndExit (std.zig)
            const result = root.main() catch |err| {
                                    ^
/usr/lib/zig/std/start.zig:232:5: 0x1187351 in _start (std.zig)
    asm volatile (switch (native_arch) {

[Direwolfesp]

fix error.DivisionByZero in ReleaseFast builds

[Direwolfesp]

thread 409072 panic: index out of bounds: index 2678, len 2678
/home/dire/Documents/Coding/github/zigtorrent/src/PiecePicker.zig:351:30: 0x10396a0 in inc_piece_refcount (zigtorrent_testing)
    std.mem.swap(u32, &pieces[old_index], &pieces[move_to]);
                             ^
/home/dire/Documents/Coding/github/zigtorrent/src/PiecePicker.zig:306:36: 0x103c8de in run (zigtorrent_testing)
        try self.inc_piece_refcount(@intCast(piece_index));
                                   ^
/home/dire/Documents/Coding/github/zigtorrent/src/main.zig:33:20: 0x102c64f in posixCallMainAndExit (zigtorrent_testing)
    try session.run();
                   ^
/usr/lib/zig/std/start.zig:232:5: 0x102b4dd in _start (zigtorrent_testing)
    asm volatile (switch (native_arch) {

[Direwolfesp]

Fix: epoll_wait is only returning one event