diff --git a/examples/endpoint/endpoint.zig b/examples/endpoint/endpoint.zig index d8a560d..3612a5b 100644 --- a/examples/endpoint/endpoint.zig +++ b/examples/endpoint/endpoint.zig @@ -65,31 +65,11 @@ fn getUser(e: *zap.SimpleEndpoint, r: zap.SimpleRequest) void { fn listUsers(e: *zap.SimpleEndpoint, r: zap.SimpleRequest) void { _ = e; - // 1MB json buffer - var jsonbuf: [1024 * 1024]u8 = undefined; - - var l: std.ArrayList(User) = std.ArrayList(User).init(alloc); - if (users.list(&l)) {} else |_| { - return; - } - var maybe_json: ?[]const u8 = null; - var maybe_free: ?std.ArrayList(u8) = null; - // if (users.count > 0) { - if (users.count > 6000) { - // if > 6000 users, 1MB might not be enough for json - // ca 45 bytes of JSON overhead + 2 * 64 bytes of data - if (zap.stringifyArrayListAlloc(alloc, User, &l, .{}) catch null) |string| { - maybe_free = string; - maybe_json = string.items; - } - } else { - maybe_json = zap.stringifyArrayListBuf(&jsonbuf, User, &l, .{}) catch null; - } - if (maybe_json) |json| { + if (users.toJSON()) |json| { _ = r.sendJson(json); - } - if (maybe_free) |free| { - free.deinit(); + alloc.free(json); + } else |err| { + std.debug.print("LIST error: {}\n", .{err}); } } @@ -105,7 +85,8 @@ fn postUser(e: *zap.SimpleEndpoint, r: zap.SimpleRequest) void { if (zap.stringifyBuf(&jsonbuf, .{ .status = "OK", .id = id }, .{})) |json| { _ = r.sendJson(json); } - } else |_| { + } else |err| { + std.debug.print("ADDING error: {}\n", .{err}); return; } } diff --git a/examples/endpoint/main.zig b/examples/endpoint/main.zig index 15e2d5b..1893053 100644 --- a/examples/endpoint/main.zig +++ b/examples/endpoint/main.zig @@ -7,6 +7,7 @@ pub fn main() !void { .thread_safe = true, }){}; var allocator = gpa.allocator(); + // setup listener var listener = zap.SimpleEndpointListener.init( allocator, @@ -14,7 +15,9 @@ pub fn main() !void { .port = 3000, .on_request = null, .log = true, - .public_folder = "./examples/endpoint/html", + .public_folder = "examples/endpoint/html", + .max_clients = 100000, + .max_body_size = 100 * 1024 * 1024, }, ); @@ -35,7 +38,7 @@ pub fn main() !void { // and run zap.start(.{ - .threads = 2, - .workers = 1, // to stay in-process, users list shared between threads + .threads = 2000, + .workers = 1, }); } diff --git a/examples/endpoint/users.zig b/examples/endpoint/users.zig index 1fe6964..4d7c5eb 100644 --- a/examples/endpoint/users.zig +++ b/examples/endpoint/users.zig @@ -1,4 +1,5 @@ const std = @import("std"); +const zap = @import("zap"); alloc: std.mem.Allocator = undefined, users: std.AutoHashMap(usize, InternalUser) = undefined, @@ -35,10 +36,12 @@ pub fn addByName(self: *Self, first: ?[]const u8, last: ?[]const u8) !usize { var user: InternalUser = undefined; user.firstnamelen = 0; user.lastnamelen = 0; + if (first) |firstname| { std.mem.copy(u8, user.firstnamebuf[0..], firstname); user.firstnamelen = firstname.len; } + if (last) |lastname| { std.mem.copy(u8, user.lastnamebuf[0..], lastname); user.lastnamelen = lastname.len; @@ -49,15 +52,26 @@ pub fn addByName(self: *Self, first: ?[]const u8, last: ?[]const u8) !usize { defer self.lock.unlock(); self.count = self.count + 1; user.id = self.count; - try self.users.put(user.id, user); - return user.id; + if (self.users.put(user.id, user)) { + return user.id; + } else |err| { + self.count -= 1; + std.debug.print("addByName error: {}\n", .{err}); + // make sure we pass on the error + return err; + } } pub fn delete(self: *Self, id: usize) bool { // We lock only on insertion, deletion, and listing self.lock.lock(); defer self.lock.unlock(); - return self.users.remove(id); + + const ret = self.users.remove(id); + if (ret) { + self.count -= 1; + } + return ret; } pub fn get(self: *Self, id: usize) ?User { @@ -80,6 +94,7 @@ pub fn update( last: ?[]const u8, ) bool { // we don't care about locking here + // we update in-place, via getPtr if (self.users.getPtr(id)) |pUser| { pUser.firstnamelen = 0; pUser.lastnamelen = 0; @@ -95,18 +110,61 @@ pub fn update( return false; } -// populate the list -pub fn list(self: *Self, out: *std.ArrayList(User)) !void { - // We lock only on insertion, deletion, and listing +pub fn toJSON(self: *Self) ![]const u8 { self.lock.lock(); defer self.lock.unlock(); - var it = JsonUserIterator.init(&self.users); + + // We create a User list that's JSON-friendly + // NOTE: we could also implement the whole JSON writing ourselves here, + // working directly with InternalUser elements of the users hashmap. + // might actually save some memory + // TODO: maybe do it directly with the user.items + var l: std.ArrayList(User) = std.ArrayList(User).init(self.alloc); + defer l.deinit(); + + var it = JsonUserIteratorWithRaceCondition.init(&self.users); + while (it.next()) |user| { + try l.append(user); + } + std.debug.assert(self.users.count() == l.items.len); + std.debug.assert(self.count == l.items.len); + return zap.stringifyArrayListAlloc(self.alloc, User, &l, .{}); +} + +// +// Note: the following code is kept in here because it taught us a lesson +// +pub fn listWithRaceCondition(self: *Self, out: *std.ArrayList(User)) !void { + // We lock only on insertion, deletion, and listing + // + // NOTE: race condition: + // ===================== + // + // the list returned from here contains elements whose slice fields + // (.first_name and .last_name) point to char buffers of elements of the + // users list: + // + // user.first_name -> internal_user.firstnamebuf[..] + // + // -> we're only referencing the memory of first and last names. + // -> while the caller works with this list, e.g. "slowly" converting it to + // JSON, the users hashmap might be added to massively in the background, + // causing it to GROW -> realloc -> all slices get invalidated! + // + // So, to mitigate that, either: + // - listing and converting to JSON must become one locked operation + // - or: the iterator must make copies of the strings + self.lock.lock(); + defer self.lock.unlock(); + var it = JsonUserIteratorWithRaceCondition.init(&self.users); while (it.next()) |user| { try out.append(user); } + std.debug.assert(self.users.count() == out.items.len); + std.debug.assert(self.count == out.items.len); } -const JsonUserIterator = struct { +const JsonUserIteratorWithRaceCondition = struct { it: std.AutoHashMap(usize, InternalUser).ValueIterator = undefined, const This = @This(); @@ -121,11 +179,23 @@ const JsonUserIterator = struct { pub fn next(this: *This) ?User { if (this.it.next()) |pUser| { - return User{ - .id = pUser.id, - .first_name = pUser.firstnamebuf[0..pUser.firstnamelen], - .last_name = pUser.lastnamebuf[0..pUser.lastnamelen], + // we get a pointer to the internal user. so it should be safe to + // create slices from its first and last name buffers + // + // SEE ABOVE NOTE regarding race condition why this is can be problematic + var user: User = .{ + // we don't need .* syntax but want to make it obvious + .id = pUser.*.id, + .first_name = pUser.*.firstnamebuf[0..pUser.*.firstnamelen], + .last_name = pUser.*.lastnamebuf[0..pUser.*.lastnamelen], }; + if (pUser.*.firstnamelen == 0) { + user.first_name = ""; + } + if (pUser.*.lastnamelen == 0) { + user.last_name = ""; + } + return user; } return null; }