Merge pull request #767 from lightpanda-io/unblock_async_http_request

Unblock async http request

Author: karlseguin

src/app.zig

@@ -52,7 +52,8 @@ pub const App = struct {
             .telemetry = undefined,
             .app_dir_path = app_dir_path,
             .notification = notification,
-            .http_client = try HttpClient.init(allocator, 5, .{
+            .http_client = try HttpClient.init(allocator, .{
+                .max_concurrent = 3,
                 .http_proxy = config.http_proxy,
                 .tls_verify_host = config.tls_verify_host,
             }),

src/browser/html/window.zig

@@ -164,7 +164,7 @@ pub const Window = struct {
     }
 
     pub fn _requestAnimationFrame(self: *Window, cbk: Function, page: *Page) !u32 {
-        return self.createTimeout(cbk, 5, page, .{.animation_frame = true});
+        return self.createTimeout(cbk, 5, page, .{ .animation_frame = true });
     }
 
     pub fn _cancelAnimationFrame(self: *Window, id: u32, page: *Page) !void {
@@ -179,7 +179,7 @@ pub const Window = struct {
 
     // TODO handle callback arguments.
     pub fn _setInterval(self: *Window, cbk: Function, delay: ?u32, page: *Page) !u32 {
-        return self.createTimeout(cbk, delay, page, .{.repeat = true});
+        return self.createTimeout(cbk, delay, page, .{ .repeat = true });
     }
 
     pub fn _clearTimeout(self: *Window, id: u32, page: *Page) !void {

src/browser/page.zig

@@ -113,7 +113,9 @@ pub const Page = struct {
             .cookie_jar = &session.cookie_jar,
             .microtask_node = .{ .func = microtaskCallback },
             .window_clicked_event_node = .{ .func = windowClicked },
-            .request_factory = browser.http_client.requestFactory(browser.notification),
+            .request_factory = browser.http_client.requestFactory(.{
+                .notification = browser.notification,
+            }),
             .scope = undefined,
             .module_map = .empty,
         };
@@ -205,58 +207,63 @@ pub const Page = struct {
         // redirect)
         self.url = request_url;
 
-        // load the data
-        var request = try self.newHTTPRequest(opts.method, &self.url, .{ .navigation = true });
-        defer request.deinit();
-        request.body = opts.body;
-        request.notification = notification;
+        {
+            // block exists to limit the lifetime of the request, which holds
+            // onto a connection
+            var request = try self.newHTTPRequest(opts.method, &self.url, .{ .navigation = true });
+            defer request.deinit();
 
-        notification.dispatch(.page_navigate, &.{
-            .opts = opts,
-            .url = &self.url,
-            .timestamp = timestamp(),
-        });
+            request.body = opts.body;
+            request.notification = notification;
 
-        var response = try request.sendSync(.{});
+            notification.dispatch(.page_navigate, &.{
+                .opts = opts,
+                .url = &self.url,
+                .timestamp = timestamp(),
+            });
 
-        // would be different than self.url in the case of a redirect
-        self.url = try URL.fromURI(arena, request.request_uri);
+            var response = try request.sendSync(.{});
 
-        const header = response.header;
-        try session.cookie_jar.populateFromResponse(&self.url.uri, &header);
+            // would be different than self.url in the case of a redirect
+            self.url = try URL.fromURI(arena, request.request_uri);
 
-        // TODO handle fragment in url.
-        try self.window.replaceLocation(.{ .url = try self.url.toWebApi(arena) });
+            const header = response.header;
+            try session.cookie_jar.populateFromResponse(&self.url.uri, &header);
 
-        const content_type = header.get("content-type");
+            // TODO handle fragment in url.
+            try self.window.replaceLocation(.{ .url = try self.url.toWebApi(arena) });
 
-        const mime: Mime = blk: {
-            if (content_type) |ct| {
-                break :blk try Mime.parse(arena, ct);
-            }
-            break :blk Mime.sniff(try response.peek());
-        } orelse .unknown;
+            const content_type = header.get("content-type");
 
-        log.info(.http, "navigation", .{
-            .status = header.status,
-            .content_type = content_type,
-            .charset = mime.charset,
-            .url = request_url,
-        });
+            const mime: Mime = blk: {
+                if (content_type) |ct| {
+                    break :blk try Mime.parse(arena, ct);
+                }
+                break :blk Mime.sniff(try response.peek());
+            } orelse .unknown;
+
+            log.info(.http, "navigation", .{
+                .status = header.status,
+                .content_type = content_type,
+                .charset = mime.charset,
+                .url = request_url,
+            });
+
+            if (!mime.isHTML()) {
+                var arr: std.ArrayListUnmanaged(u8) = .{};
+                while (try response.next()) |data| {
+                    try arr.appendSlice(arena, try arena.dupe(u8, data));
+                }
+                // save the body into the page.
+                self.raw_data = arr.items;
+                return;
+            }
 
-        if (mime.isHTML()) {
-            self.raw_data = null;
             try self.loadHTMLDoc(&response, mime.charset orelse "utf-8");
-            try self.processHTMLDoc();
-        } else {
-            var arr: std.ArrayListUnmanaged(u8) = .{};
-            while (try response.next()) |data| {
-                try arr.appendSlice(arena, try arena.dupe(u8, data));
-            }
-            // save the body into the page.
-            self.raw_data = arr.items;
         }
 
+        try self.processHTMLDoc();
+
         notification.dispatch(.page_navigated, &.{
             .url = &self.url,
             .timestamp = timestamp(),

src/browser/session.zig

@@ -72,7 +72,7 @@ pub const Session = struct {
 
     pub fn deinit(self: *Session) void {
         if (self.page != null) {
-            self.removePage();
+            self.removePage() catch {};
         }
         self.cookie_jar.deinit();
         self.storage_shed.deinit();
@@ -104,14 +104,35 @@ pub const Session = struct {
         return page;
     }
 
-    pub fn removePage(self: *Session) void {
+    pub fn removePage(self: *Session) !void {
         // Inform CDP the page is going to be removed, allowing other worlds to remove themselves before the main one
         self.browser.notification.dispatch(.page_remove, .{});
 
         std.debug.assert(self.page != null);
-        // Reset all existing callbacks.
-        self.browser.app.loop.reset();
+
+        // Cleanup is a bit sensitive. We could still have inflight I/O. For
+        // example, we could have an XHR request which is still in the connect
+        // phase. It's important that we clean these up, as they're holding onto
+        // limited resources (like our fixed-sized http state pool).
+        //
+        // First thing we do, is endScope() which will execute the destructor
+        // of any type that registered a destructor (e.g. XMLHttpRequest).
+        // This will shutdown any pending sockets, which begins our cleaning
+        // processed
         self.executor.endScope();
+
+        // Second thing we do is reset the loop. This increments the loop ctx_id
+        // so that any "stale" timeouts we process will get ignored. We need to
+        // do this BEFORE running the loop because, at this point, things like
+        // window.setTimeout and running microtasks should be ignored
+        self.browser.app.loop.reset();
+
+        // Finally, we run the loop. Because of the reset just above, this will
+        // ignore any timeouts. And, because of the endScope about this, it
+        // should ensure that the http requests detect the shutdown socket and
+        // release their resources.
+        try self.browser.app.loop.run();
+
         self.page = null;
 
         // clear netsurf memory arena.
@@ -143,7 +164,7 @@ pub const Session = struct {
         // the final URL, possibly following redirects)
         const url = try self.page.?.url.resolve(self.transfer_arena, url_string);
 
-        self.removePage();
+        try self.removePage();
         var page = try self.createPage();
         return page.navigate(url, opts);
     }

src/browser/xhr/xhr.zig

@@ -30,6 +30,7 @@ const Mime = @import("../mime.zig").Mime;
 const parser = @import("../netsurf.zig");
 const http = @import("../../http/client.zig");
 const Page = @import("../page.zig").Page;
+const Loop = @import("../../runtime/loop.zig").Loop;
 const CookieJar = @import("../storage/storage.zig").CookieJar;
 
 // XHR interfaces
@@ -78,6 +79,7 @@ const XMLHttpRequestBodyInit = union(enum) {
 
 pub const XMLHttpRequest = struct {
     proto: XMLHttpRequestEventTarget = XMLHttpRequestEventTarget{},
+    loop: *Loop,
     arena: Allocator,
     request: ?*http.Request = null,
 
@@ -91,6 +93,7 @@ pub const XMLHttpRequest = struct {
     sync: bool = true,
     err: ?anyerror = null,
     last_dispatch: i64 = 0,
+    request_body: ?[]const u8 = null,
 
     cookie_jar: *CookieJar,
     // the URI of the page where this request is originating from
@@ -241,12 +244,13 @@ pub const XMLHttpRequest = struct {
     pub fn constructor(page: *Page) !XMLHttpRequest {
         const arena = page.arena;
         return .{
+            .url = null,
             .arena = arena,
+            .loop = page.loop,
             .headers = Headers.init(arena),
             .response_headers = Headers.init(arena),
             .method = undefined,
             .state = .unsent,
-            .url = null,
             .origin_url = &page.url,
             .cookie_jar = page.cookie_jar,
         };
@@ -422,18 +426,31 @@ pub const XMLHttpRequest = struct {
         log.debug(.http, "request", .{ .method = self.method, .url = self.url, .source = "xhr" });
 
         self.send_flag = true;
+        if (body) |b| {
+            self.request_body = try self.arena.dupe(u8, b);
+        }
 
-        self.request = try page.request_factory.create(self.method, &self.url.?.uri);
-        var request = self.request.?;
-        errdefer request.deinit();
+        try page.request_factory.initAsync(
+            page.arena,
+            self.method,
+            &self.url.?.uri,
+            self,
+            onHttpRequestReady,
+            self.loop,
+        );
+    }
+
+    fn onHttpRequestReady(ctx: *anyopaque, request: *http.Request) !void {
+        // on error, our caller will cleanup request
+        const self: *XMLHttpRequest = @alignCast(@ptrCast(ctx));
 
         for (self.headers.list.items) |hdr| {
             try request.addHeader(hdr.name, hdr.value, .{});
         }
 
         {
             var arr: std.ArrayListUnmanaged(u8) = .{};
-            try self.cookie_jar.forRequest(&self.url.?.uri, arr.writer(page.arena), .{
+            try self.cookie_jar.forRequest(&self.url.?.uri, arr.writer(self.arena), .{
                 .navigation = false,
                 .origin_uri = &self.origin_url.uri,
             });
@@ -447,14 +464,15 @@ pub const XMLHttpRequest = struct {
         //  if the request method is GET or HEAD.
         //  https://xhr.spec.whatwg.org/#the-send()-method
         // var used_body: ?XMLHttpRequestBodyInit = null;
-        if (body) |b| {
+        if (self.request_body) |b| {
             if (self.method != .GET and self.method != .HEAD) {
-                request.body = try page.arena.dupe(u8, b);
+                request.body = b;
                 try request.addHeader("Content-Type", "text/plain; charset=UTF-8", .{});
             }
         }
 
-        try request.sendAsync(page.loop, self, .{});
+        try request.sendAsync(self.loop, self, .{});
+        self.request = request;
     }
 
     pub fn onHttpResponse(self: *XMLHttpRequest, progress_: anyerror!http.Progress) !void {

src/cdp/domains/input.zig

@@ -90,7 +90,7 @@ fn clickNavigate(cmd: anytype, uri: std.Uri) !void {
         .disposition = "currentTab",
     }, .{ .session_id = bc.session_id.? });
 
-    bc.session.removePage();
+    try bc.session.removePage();
     _ = try bc.session.createPage(null);
 
     try @import("page.zig").navigateToUrl(cmd, url, false);

src/cdp/domains/target.zig

@@ -220,7 +220,7 @@ fn closeTarget(cmd: anytype) !void {
         bc.session_id = null;
     }
 
-    bc.session.removePage();
+    try bc.session.removePage();
     if (bc.isolated_world) |*world| {
         world.deinit();
         bc.isolated_world = null;