Skip to content

Commit

Permalink
Add a separate arena for HTTPConn
Browse files Browse the repository at this point in the history 
We now have two arenas:
a - for things that exist throughout the lifetime of a conn
b - for things that exist throughout the lifetime of a single request

We already had (b), which is the more important one. (a) just helps make cleanup
easier.
  • Loading branch information
@karlseguin
karlseguin committed Sep 2, 2024
1 parent c95c9ad commit 2b97d1e
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 78 deletions.
2 changes: 1 addition & 1 deletion src/httpz.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ pub fn Server(comptime H: type) type {
// because the response data must outlive the execution of this function
// (and thus, in nonblocking, outlives this threadpool's execution unit).
pub fn handleRequest(self: *Self, conn: *HTTPConn, thread_buf: []u8) void {
const aa = conn.arena.allocator();
const aa = conn.req_arena.allocator();

var fba = FixedBufferAllocator.init(thread_buf);
var fb = FallbackAllocator{
Expand Down
76 changes: 31 additions & 45 deletions src/request.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const KeyValue = @import("key_value.zig").KeyValue;
const MultiFormKeyValue = @import("key_value.zig").MultiFormKeyValue;
const Config = @import("config.zig").Config.Request;

const Stream = std.net.Stream;
const Address = std.net.Address;
const Allocator = std.mem.Allocator;
const ArenaAllocator = std.heap.ArenaAllocator;
Expand Down Expand Up @@ -75,8 +74,6 @@ pub const Request = struct {
middlewares: *std.StringHashMap(*anyopaque),

pub const State = Self.State;
pub const Config = Self.Config;
pub const Reader = Self.Reader;

pub fn init(arena: Allocator, conn: *HTTPConn) Request {
const state = &conn.req_state;
Expand Down Expand Up @@ -524,48 +521,37 @@ pub const State = struct {
// know what it is from the content-length header
body_len: usize,

arena: *ArenaAllocator,

middlewares: std.StringHashMap(*anyopaque),

const asUint = @import("url.zig").asUint;

pub fn init(allocator: Allocator, arena: *ArenaAllocator, buffer_pool: *buffer.Pool, config: *const Config) !Request.State {
pub fn init(arena: Allocator, buffer_pool: *buffer.Pool, config: *const Config) !Request.State {
return .{
.pos = 0,
.len = 0,
.url = null,
.method = null,
.protocol = null,
.body = null,
.body_pos = 0,
.body_len = 0,
.arena = arena,
.method = null,
.protocol = null,
.buffer_pool = buffer_pool,
.max_body_size = config.max_body_size orelse 1_048_576,
.middlewares = std.StringHashMap(*anyopaque).init(allocator),
.qs = try KeyValue.init(allocator, config.max_query_count orelse 32),
.fd = try KeyValue.init(allocator, config.max_form_count orelse 0),
.mfd = try MultiFormKeyValue.init(allocator, config.max_multiform_count orelse 0),
.buf = try allocator.alloc(u8, config.buffer_size orelse 4_096),
.headers = try KeyValue.init(allocator, config.max_header_count orelse 32),
.params = try Params.init(allocator, config.max_param_count orelse 10),
.middlewares = std.StringHashMap(*anyopaque).init(arena),
.qs = try KeyValue.init(arena, config.max_query_count orelse 32),
.fd = try KeyValue.init(arena, config.max_form_count orelse 0),
.mfd = try MultiFormKeyValue.init(arena, config.max_multiform_count orelse 0),
.buf = try arena.alloc(u8, config.buffer_size orelse 4_096),
.headers = try KeyValue.init(arena, config.max_header_count orelse 32),
.params = try Params.init(arena, config.max_param_count orelse 10),
};
}

pub fn deinit(self: *State, allocator: Allocator) void {
// not our job to clear the arena!
pub fn deinit(self: *State) void {
if (self.body) |buf| {
self.buffer_pool.release(buf);
self.body = null;
}
allocator.free(self.buf);
self.qs.deinit(allocator);
self.fd.deinit(allocator);
self.mfd.deinit(allocator);
self.params.deinit(allocator);
self.headers.deinit(allocator);
self.middlewares.deinit();
}

pub fn reset(self: *State) void {
Expand All @@ -592,7 +578,7 @@ pub const State = struct {
}

// returns true if the header has been fully parsed
pub fn parse(self: *State, stream: anytype) !bool {
pub fn parse(self: *State, req_arena: Allocator, stream: anytype) !bool {
if (self.body != null) {
// if we have a body, then we've read the header. We want to read into
// self.body, not self.buf.
Expand All @@ -609,13 +595,13 @@ pub const State = struct {
self.len = len;

if (self.method == null) {
if (try self.parseMethod(buf[0..len])) return true;
if (try self.parseMethod(req_arena, buf[0..len])) return true;
} else if (self.url == null) {
if (try self.parseUrl(buf[self.pos..len])) return true;
if (try self.parseUrl(req_arena, buf[self.pos..len])) return true;
} else if (self.protocol == null) {
if (try self.parseProtocol(buf[self.pos..len])) return true;
if (try self.parseProtocol(req_arena, buf[self.pos..len])) return true;
} else {
if (try self.parseHeaders(buf[self.pos..len])) return true;
if (try self.parseHeaders(req_arena, buf[self.pos..len])) return true;
}

if (self.body == null and len == buf.len) {
Expand All @@ -625,7 +611,7 @@ pub const State = struct {
return false;
}

fn parseMethod(self: *State, buf: []u8) !bool {
fn parseMethod(self: *State, req_arena: Allocator, buf: []u8) !bool {
const buf_len = buf.len;

// Shortest method is only 3 characters (+1 trailing space), so
Expand Down Expand Up @@ -677,10 +663,10 @@ pub const State = struct {
else => return error.UnknownMethod,
}

return try self.parseUrl(buf[self.pos..]);
return try self.parseUrl(req_arena, buf[self.pos..]);
}

fn parseUrl(self: *State, buf: []u8) !bool {
fn parseUrl(self: *State, req_arena: Allocator, buf: []u8) !bool {
const buf_len = buf.len;
if (buf_len == 0) return false;

Expand All @@ -706,10 +692,10 @@ pub const State = struct {
}

self.pos += len;
return self.parseProtocol(buf[len..]);
return self.parseProtocol(req_arena, buf[len..]);
}

fn parseProtocol(self: *State, buf: []u8) !bool {
fn parseProtocol(self: *State, req_arena: Allocator, buf: []u8) !bool {
const buf_len = buf.len;
if (buf_len < 10) return false;

Expand All @@ -728,10 +714,10 @@ pub const State = struct {
}

self.pos += 10;
return try self.parseHeaders(buf[10..]);
return try self.parseHeaders(req_arena, buf[10..]);
}

fn parseHeaders(self: *State, full: []u8) !bool {
fn parseHeaders(self: *State, req_arena: Allocator, full: []u8) !bool {
var buf = full;
var headers = &self.headers;
line: while (buf.len > 0) {
Expand Down Expand Up @@ -802,7 +788,7 @@ pub const State = struct {
if (buf[1] == '\n') {
// we have \r\n at the start of a line, we're done
self.pos += 2;
return try self.prepareForBody();
return try self.prepareForBody(req_arena);
}
// we have a \r followed by something that isn't a \n, can't be right
return error.InvalidHeaderLine;
Expand All @@ -818,7 +804,7 @@ pub const State = struct {
}

// we've finished reading the header
fn prepareForBody(self: *State) !bool {
fn prepareForBody(self: *State, req_arena: Allocator) !bool {
const str = self.headers.get("content-length") orelse return true;
const cl = atoi(str) orelse return error.InvalidContentLength;

Expand Down Expand Up @@ -861,7 +847,7 @@ pub const State = struct {
self.pos = len + missing;
} else {
// We don't have the [full] body, and our static buffer is too small
const body_buf = try self.buffer_pool.arenaAlloc(self.arena.allocator(), cl);
const body_buf = try self.buffer_pool.arenaAlloc(req_arena, cl);
@memcpy(body_buf.data[0..read], buf[pos .. pos + read]);
self.body = body_buf;
}
Expand Down Expand Up @@ -1517,11 +1503,11 @@ test "request: fuzz" {
var conn = ctx.conn;
var fake_reader = ctx.fakeReader();
while (true) {
const done = try conn.req_state.parse(&fake_reader);
const done = try conn.req_state.parse(conn.req_arena.allocator(), &fake_reader);
if (done) break;
}

var request = Request.init(conn.arena.allocator(), conn);
var request = Request.init(conn.req_arena.allocator(), conn);

// assert the headers
var it = headers.iterator();
Expand Down Expand Up @@ -1550,18 +1536,18 @@ fn testParse(input: []const u8, config: Config) !Request {
var ctx = t.Context.allocInit(t.arena.allocator(), .{ .request = config });
ctx.write(input);
while (true) {
const done = try ctx.conn.req_state.parse(ctx.stream);
const done = try ctx.conn.req_state.parse(ctx.conn.req_arena.allocator(), ctx.stream);
if (done) break;
}
return Request.init(ctx.conn.arena.allocator(), ctx.conn);
return Request.init(ctx.conn.req_arena.allocator(), ctx.conn);
}

fn expectParseError(expected: anyerror, input: []const u8, config: Config) !void {
var ctx = t.Context.init(.{ .request = config });
defer ctx.deinit();

ctx.write(input);
try t.expectError(expected, ctx.conn.req_state.parse(ctx.stream));
try t.expectError(expected, ctx.conn.req_state.parse(ctx.conn.req_arena.allocator(), ctx.stream));
}

fn randomMethod(random: std.Random) []const u8 {
Expand Down
24 changes: 11 additions & 13 deletions src/t.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
const std = @import("std");
const httpz = @import("httpz.zig");

const Allocator = std.mem.Allocator;

const Conn = @import("worker.zig").HTTPConn;
const BufferPool = @import("buffer.zig").Pool;

Expand Down Expand Up @@ -46,7 +48,7 @@ pub const Context = struct {
to_read: std.ArrayList(u8),
_random: ?std.Random.DefaultPrng = null,

pub fn allocInit(ctx_allocator: std.mem.Allocator, config_: httpz.Config) Context {
pub fn allocInit(ctx_allocator: Allocator, config_: httpz.Config) Context {
var pair: [2]c_int = undefined;
const rc = std.c.socketpair(std.posix.AF.LOCAL, std.posix.SOCK.STREAM, 0, &pair);
if (rc != 0) {
Expand All @@ -73,6 +75,7 @@ pub const Context = struct {

var ctx_arena = ctx_allocator.create(std.heap.ArenaAllocator) catch unreachable;
ctx_arena.* = std.heap.ArenaAllocator.init(ctx_allocator);

const aa = ctx_arena.allocator();

const bp = aa.create(BufferPool) catch unreachable;
Expand All @@ -90,13 +93,7 @@ pub const Context = struct {
if (cw.large_buffer_size == null) config.workers.large_buffer_size = 256;
}

// a bit over the top..we could use ctx_arena here, but this better mimics
// the actual code where the conn has a distinct arena (whereas ctx_arena is
// something specific to this test context)
const conn_arena = aa.create(std.heap.ArenaAllocator) catch unreachable;
conn_arena.* = std.heap.ArenaAllocator.init(aa);

const req_state = httpz.Request.State.init(aa, conn_arena, bp, &config.request) catch unreachable;
const req_state = httpz.Request.State.init(aa, bp, &config.request) catch unreachable;
const res_state = httpz.Response.State.init(aa, &config.response) catch unreachable;

const conn = aa.create(Conn) catch unreachable;
Expand All @@ -110,8 +107,9 @@ pub const Context = struct {
.timeout = 0,
.request_count = 0,
.close = false,
.arena = conn_arena,
.ws_worker = undefined,
.conn_arena = ctx_arena,
.req_arena = std.heap.ArenaAllocator.init(aa),
};

return .{
Expand Down Expand Up @@ -158,7 +156,7 @@ pub const Context = struct {
}
}

pub fn read(self: Context, a: std.mem.Allocator) !std.ArrayList(u8) {
pub fn read(self: Context, a: Allocator) !std.ArrayList(u8) {
var buf: [1024]u8 = undefined;
var arr = std.ArrayList(u8).init(a);

Expand Down Expand Up @@ -231,11 +229,11 @@ pub const Context = struct {
}

pub fn request(self: Context) httpz.Request {
return httpz.Request.init(self.conn.arena.allocator(), self.conn);
return httpz.Request.init(self.conn.req_arena.allocator(), self.conn);
}

pub fn response(self: Context) httpz.Response {
return httpz.Response.init(self.conn.arena.allocator(), self.conn);
return httpz.Response.init(self.conn.req_arena.allocator(), self.conn);
}

pub fn reset(self: Context) void {
Expand Down Expand Up @@ -271,7 +269,7 @@ pub const Context = struct {
};
};

pub fn randomString(random: std.Random, a: std.mem.Allocator, max: usize) []u8 {
pub fn randomString(random: std.Random, a: Allocator, max: usize) []u8 {
var buf = a.alloc(u8, random.uintAtMost(usize, max) + 1) catch unreachable;
const valid = "abcdefghijklmnopqrstuvwxyz0123456789-_";
for (0..buf.len) |i| {
Expand Down
4 changes: 2 additions & 2 deletions src/testing.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ pub fn init(config: httpz.Config) Testing {
// thereafter to change whatever properties they want.
var base_request = std.io.fixedBufferStream("GET / HTTP/1.1\r\nContent-Length: 0\r\n\r\n");
while (true) {
const done = conn.req_state.parse(&base_request) catch unreachable;
const done = conn.req_state.parse(conn.req_arena.allocator(), &base_request) catch unreachable;
if (done) {
break;
}
}

const aa = conn.arena.allocator();
const aa = conn.req_arena.allocator();

const req = aa.create(httpz.Request) catch unreachable;
req.* = ctx.request();
Expand Down
Loading

0 comments on commit 2b97d1e

Please sign in to comment.