Skip to content

Fix memory leak #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nickhendo wants to merge 8 commits into from
Closed

Fix memory leak #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4ea7804
Explicitly detach from threads
nickhendo Oct 27, 2025
bb45aef
Update GA for testing
nickhendo Oct 27, 2025
9967547
Push to test pypi
nickhendo Oct 27, 2025
6f9241e
Rename zig build in makefile
nickhendo Oct 29, 2025
9b87900
Create Router and pass by pointer
nickhendo Oct 29, 2025
b8f2355
Defer explicit free and destroy
nickhendo Oct 29, 2025
a5d026a
Move to threadpool
nickhendo Oct 29, 2025
3b6cb7e
Rename
nickhendo Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/pypi.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ name: Build and Publish Wheels

on:
push:
branches: [main]
branches:
- main
- 11-memory-leak
release:
types: [published]
workflow_dispatch: # Allows manual triggering
Expand All @@ -16,8 +18,6 @@ jobs:
os:
- ubuntu-latest
- macos-latest
# - macos-13 # for x86 support # Not supported, takes too long to build in GA
# - windows-latest # Not supported, zig code needs rework

steps:
- uses: actions/checkout@v5
Expand Down Expand Up @@ -95,7 +95,7 @@ jobs:
url: https://test.pypi.org/project/volt-framework/
permissions:
id-token: write
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
if: github.event_name == 'push'

steps:
- uses: actions/download-artifact@v4
Expand Down
12 changes: 6 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
all: build
all: build-zig

build:
zig build -Doptimize=Debug -freference-trace
build-zig:
zig build -Doptimize=Debug -freference-trace --prefix volt

run:
-pkill -TERM -f "python example.py" && sleep 2 || pkill -KILL -f "python example.py"
Expand All @@ -15,13 +15,13 @@ debug-test:
-pkill -TERM -f "python example.py" && sleep 2 || pkill -KILL -f "python example.py"
lldb pytest

test: build
test: build-zig
@echo "Runnning Zig tests..."
zig test src/volt.zig
@echo "Runnning Python tests..."
NO_LOGS="true" pytest

test-verbose: build
test-verbose: build-zig
zig test src/volt.zig
pytest -svv

Expand All @@ -31,7 +31,7 @@ inspect-coredump:
generate:
python -m volt.cli generate

watch: build generate run
watch: build-zig generate run
watchman-make \
-p '**/*.zig' -t build run \
-p '**/*.html' '**/*.js' -t tailwind \
Expand Down
1 change: 1 addition & 0 deletions build.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub fn build(b: *std.Build) void {
.root_source_file = b.path("src/volt.zig"),
.target = target,
.optimize = optimize,
.single_threaded = false,
});

// We will also create a module for our other entry point, 'main.zig'.
Expand Down
71 changes: 23 additions & 48 deletions src/volt.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,25 +56,25 @@ pub export fn run_server(
if (std.posix.getenv("NO_LOGS") != null) {
no_logs = true;
}

const routes = registerRoutes(arena_allocator, routes_to_register, num_routes) catch |err| {
log.err("error registering routes: {any}", .{err});
return;
};

runServer(allocator, server_addr, server_port, routes, 0, &should_exit) catch |err| {
var router = Router.init(arena_allocator, routes);

runServer(allocator, server_addr, server_port, &router, &should_exit) catch |err| {
log.err("error running server: {any}", .{err});
return;
};
}

/// stop_iter will stop the server after stop_iter iterations, unless stop_iter is 0. This is for testing,
/// so as to prevent blocking
fn runServer(
allocator: std.mem.Allocator,
server_addr: [*:0]const u8,
server_port: u16,
routes: []Route,
stop_iter: u16,
router: *Router,
exit: *bool,
) !void {
const server_addr_slice = std.mem.span(server_addr);
Expand Down Expand Up @@ -114,16 +114,18 @@ fn runServer(

server_is_running = true;

var num_iters: u16 = 0;
var threadPool: std.Thread.Pool = undefined;
try threadPool.init(std.Thread.Pool.Options{
.allocator = allocator,
.n_jobs = 1,
.stack_size = std.Thread.SpawnConfig.default_stack_size,
.track_ids = false,
});
defer threadPool.deinit();

// Continue checking for new connections. New connections are given a separate thread to be handled in.
// This thread will continue waiting for requests on the same connection until the connection is closed.
while (!exit.*) {
if (stop_iter > 0 and num_iters >= stop_iter) {
break;
}
if (stop_iter > 0) {
num_iters += 1;
}
const connection = server.accept() catch |err| {
if (err == error.WouldBlock) {
std.Thread.sleep(10 * std.time.ns_per_ms);
Expand All @@ -137,45 +139,17 @@ fn runServer(
log.debug("Handling new connection", .{});

// Give each new connection a new thread.
// TODO: This should probably be a threadpool, and the closure of threads handled properly
_ = std.Thread.spawn(
.{},
try threadPool.spawn(
handleConnection,
.{ allocator, connection, routes },
) catch |err| {
log.err("failed to spawn thread: {any}", .{err});
continue;
};
.{ allocator, &connection, router },
);
log.debug("Thread spawned", .{});
}

log.info("Shutting down...", .{});
}

/// Function to wrap runServer for use in threads, since error returning functions can't be used
/// as thread functions. Panics on err, which is fine, since this is used for testing only
fn runServerWithErrorHandler(
allocator: std.mem.Allocator,
server_addr: [*:0]const u8,
server_port: u16,
routes: []Route,
stop_iter: u16,
exit: *bool,
) void {
runServer(
allocator,
server_addr,
server_port,
routes,
stop_iter,
exit,
) catch |err| {
log.err("error running server: {any}", .{err});
@panic("error running server in runServerWithErrorHandler");
};
}

fn handleConnection(allocator: std.mem.Allocator, connection: std.net.Server.Connection, routes: []Route) void {
fn handleConnection(allocator: std.mem.Allocator, connection: *const std.net.Server.Connection, router: *Router) void {
var recv_header: [4000]u8 = undefined;
var send_header: [4000]u8 = undefined;
var conn_reader = connection.stream.reader(&recv_header);
Expand Down Expand Up @@ -223,7 +197,7 @@ fn handleConnection(allocator: std.mem.Allocator, connection: std.net.Server.Con
const head = request.head;

const logging_middleware = middleware.Logging.init();
const status = handleRequest(allocator, routes, &request) catch |err| {
const status = handleRequest(allocator, router, &request) catch |err| {
log.err("Error calling handleRequest in handleConnection(): {}", .{err});
if (@errorReturnTrace()) |trace| {
std.debug.dumpStackTrace(trace.*);
Expand Down Expand Up @@ -259,7 +233,7 @@ fn registerRoutes(arena: std.mem.Allocator, routes_to_register: [*]const http.Ro
return error.BadMethod;
}
routes[i].method = method.?;
routes[i].path = try arena.dupeZ(u8, std.mem.span(routes_to_register[i].name));
routes[i].path = std.mem.span(routes_to_register[i].name);
routes[i].handler = routes_to_register[i].handler;

log.debug("zig: Route registered: {s} -> {any}", .{ routes[i].path, routes[i].handler });
Expand Down Expand Up @@ -385,7 +359,7 @@ fn handleStaticRoute(request: *std.http.Server.Request) !std.http.Status {
return status;
}

fn handleRequest(allocator: std.mem.Allocator, routes: []Route, request: *std.http.Server.Request) !std.http.Status {
fn handleRequest(allocator: std.mem.Allocator, router: *Router, request: *std.http.Server.Request) !std.http.Status {
log.debug("Handling request for {s}", .{request.head.target});

// CORS middleware will respond to request if allowed is false
Expand All @@ -404,7 +378,6 @@ fn handleRequest(allocator: std.mem.Allocator, routes: []Route, request: *std.ht
defer arena.deinit();
const arena_allocator = arena.allocator();

var router = Router.init(arena_allocator, routes);
var matched_route = try router.match(request.head.method, request.head.target);
// const route = routing.getRoute(routes, request.head.target);
if (matched_route == null) {
Expand Down Expand Up @@ -478,6 +451,8 @@ fn handleRequest(allocator: std.mem.Allocator, routes: []Route, request: *std.ht

var response: *http.Response = undefined;
const success = handler(&req, &response, &context);
defer arena_allocator.free(response.headers);
defer arena_allocator.destroy(response);
log.debug("handler complete", .{});
if (success == 0) {
log.err("handler was unsuccessful", .{});
Expand Down
132 changes: 0 additions & 132 deletions volt/components.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,135 +119,3 @@ def __init__(self, block_name: str, template_name: str, message: str | None = No
message or f"Block {block_name} not in template {template_name}"
)


#
# # NOTE: Not sure how _this_ would be generated
# class NavSelected(StrEnum):
# HOME = "home"
# FEATURES = "features"
# DEMO = "demo"
# PERFORMANCE = "performance"
# QUICKSTART = "quickstart"
#
#
# class NavBar(Component):
# template_name: str = "base.html"
# block_name: str = "navbar"
#
# @dataclass
# class Context(Component.Context):
# selected: NavSelected
#
# def __init__(self, context: Context) -> None:
# super().__init__(context)
#
#
# class Base(Component):
# template_name: str = "base.html"
#
# @dataclass
# class Context(Component.Context):
# selected: NavSelected
#
# def __init__(self, context: Context) -> None:
# super().__init__(context)
#
#
# # vvv Tentatively thinking these component classes should be generated? vvv #
# class Home(Base):
# template_name: str = "home.html"
#
# @dataclass
# class Context(Base.Context): ...
#
# def __init__(self, context: Context) -> None:
# super().__init__(context)
#
#
# class Features(Base):
# template_name: str = "features.html"
#
# @dataclass
# class Context(Base.Context): ...
#
# def __init__(self, context: Context) -> None:
# super().__init__(context)
#
#
# @dataclass
# class ProgrammingLanguage:
# name: str
# abbrev: str
# description: str
# category: str
# text_colour: str
# bg_colour: str
#
#
# class DemoLanguages(Component):
# template_name: str = "demo.html"
# block_name: str = "programming_language_list"
#
# @dataclass
# class Context(Component.Context):
# programming_languages: list[ProgrammingLanguage]
# searching: bool
#
# def __init__(self, context: Context) -> None:
# super().__init__(context)
#
#
# class DemoCounter(Component):
# template_name: str = "demo.html"
# block_name: str = "counter"
#
# @dataclass
# class Context(Component.Context):
# value: int
#
# def __init__(self, context: Context) -> None:
# super().__init__(context)
#
#
# class DemoTasks(Component):
# template_name: str = "demo.html"
# block_name: str = "task_list"
#
# @dataclass
# class Context(Component.Context):
# tasks: list[str]
#
# def __init__(self, context: Context) -> None:
# super().__init__(context)
#
# @dataclass
# class ChatMessage:
# message: str
# time: datetime
#
# class DemoChatMessages(Component):
# template_name: str = "demo.html"
# block_name: str = "chat_messages"
#
# @dataclass
# class Context(Component.Context):
# messages: list[ChatMessage]
#
# def __init__(self, context: Context) -> None:
# super().__init__(context)
#
# class Demo(Base):
# template_name: str = "demo.html"
#
# @dataclass
# class Context(
# Base.Context,
# DemoLanguages.Context,
# DemoCounter.Context,
# DemoTasks.Context,
# ):
# tasks: list[str]
# value: int
#
# def __init__(self, context: Context) -> None:
# super().__init__(context)
Loading