Compare commits

...

4 Commits

Author SHA1 Message Date
9c3fcc5a9d
address memory leaks in awshttp
There appears to be a leak in json.zig, which had minimal modifications for
snake/camel case, including use of an allocator. It is not obvious that
the leaks were added by these changes, so I will leave this issue for
later investigation.
2021-06-18 14:06:58 -07:00
667c38c8d3
add sentinals so we can work until general purpose allocator 2021-06-18 13:48:31 -07:00
2768c83a0a
switch to general purpose allocator so we can weed out leaks 2021-06-18 13:47:27 -07:00
1050ca5222
update documentation and comments 2021-06-18 13:46:23 -07:00
4 changed files with 58 additions and 28 deletions

View File

@ -95,15 +95,14 @@ TODO List:
* ✓ Implement generic response body -> Response type handling (right now, this is hard-coded) * ✓ Implement generic response body -> Response type handling (right now, this is hard-coded)
* ✓ Implement codegen for services with xml structures (using Smithy models) * ✓ Implement codegen for services with xml structures (using Smithy models)
* ✓ Implement codegen for others (using Smithy models) * ✓ Implement codegen for others (using Smithy models)
* Switch to aws-c-cal upsream once [PR for full static musl build support is merged](https://github.com/awslabs/aws-c-cal/pull/89) (see Dockerfile) * Switch to aws-c-cal upstream once [PR for full static musl build support is merged](https://github.com/awslabs/aws-c-cal/pull/89) (see Dockerfile)
* Remove compiler 0.7.1 shims when 0.8.0 is released * Remove compiler 0.7.1 shims when 0.8.0 is released
(new 2021-05-29. I will proceed in this order unless I get other requests) (new 2021-05-29. I will proceed in this order unless I get other requests)
* Implement [AWS query protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-query-protocol.html). This is the protocol in use by sts.getcalleridentity * ✓ Implement [AWS query protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-query-protocol.html). This is the protocol in use by sts.getcalleridentity. Total service count 18
* Implement [AWS Json 1.0 protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-json-1_0-protocol.html) * Implement [AWS Json 1.0 protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-json-1_0-protocol.html). Includes dynamodb. Total service count 18
* Implement [AWS Json 1.1 protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-json-1_1-protocol.html) * Implement [AWS Json 1.1 protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-json-1_1-protocol.html). Includes ecs. Total service count 105
* Implement [AWS restXml protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-restxml-protocol.html) * Implement [AWS restXml protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-restxml-protocol.html). Includes S3. Total service count 4.
* Implement [AWS EC2 query protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-ec2-query-protocol.html) * ✓ Implement [AWS EC2 query protocol](https://awslabs.github.io/smithy/1.0/spec/aws/aws-ec2-query-protocol.html). Includes EC2. Total service count 1.
Compiler wishlist/watchlist: Compiler wishlist/watchlist:

View File

@ -55,22 +55,32 @@ pub const Aws = struct {
}); });
log.debug("proto: {s}", .{service.aws_protocol}); log.debug("proto: {s}", .{service.aws_protocol});
// It seems as though there are 3 major branches of the 6 protocols.
// 1. query/ec2_query, which are identical until you get to complex
// structures. TBD if the shortcut we're taking for query to make
// it return json will work on EC2, but my guess is yes.
// 2. *json*: These three appear identical for input (possible difference
// for empty body serialization), but differ in error handling.
// We're not doing a lot of error handling here, though.
// 3. rest_xml: This is a one-off for S3, never used since
switch (service.aws_protocol) { switch (service.aws_protocol) {
.query => return self.callQuery(request, service, action, options), .query, .ec2_query => return self.callQuery(request, service, action, options),
.ec2_query => @compileError("EC2 Query protocol not yet supported"), .rest_json_1, .json_1_0, .json_1_1 => @compileError("REST Json, Json 1.0/1.1 protocol not yet supported"),
.rest_json_1 => @compileError("REST Json 1 protocol not yet supported"),
.json_1_0 => @compileError("Json 1.0 protocol not yet supported"),
.json_1_1 => @compileError("Json 1.1 protocol not yet supported"),
.rest_xml => @compileError("REST XML protocol not yet supported"), .rest_xml => @compileError("REST XML protocol not yet supported"),
} }
} }
// Call using query protocol. This is documented as an XML protocol, but // Call using query protocol. This is documented as an XML protocol, but
// throwing a JSON accept header seems to work // throwing a JSON accept header seems to work. EC2Query is very simliar to
// Query, so we'll handle both here. Realistically we probably don't effectively
// handle lists and maps properly anyway yet, so we'll go for it and see
// where it breaks. PRs and/or failing test cases appreciated.
fn callQuery(self: Self, comptime request: anytype, service: anytype, action: anytype, options: Options) !FullResponse(request) { fn callQuery(self: Self, comptime request: anytype, service: anytype, action: anytype, options: Options) !FullResponse(request) {
var buffer = std.ArrayList(u8).init(self.allocator); var buffer = std.ArrayList(u8).init(self.allocator);
defer buffer.deinit(); defer buffer.deinit();
const writer = buffer.writer(); const writer = buffer.writer();
// TODO: transformation function should be refactored for operation
// with a Writer passed in so we don't have to allocate
const transformer = struct { const transformer = struct {
allocator: *std.mem.Allocator, allocator: *std.mem.Allocator,
@ -98,6 +108,7 @@ pub const Aws = struct {
.sigv4_service_name = service.sigv4_name, .sigv4_service_name = service.sigv4_name,
}, },
); );
// TODO: Can response handling be reused?
defer response.deinit(); defer response.deinit();
if (response.response_code != 200) { if (response.response_code != 200) {
log.err("call failed! return status: {d}", .{response.response_code}); log.err("call failed! return status: {d}", .{response.response_code});

View File

@ -77,7 +77,10 @@ const SigningOptions = struct {
const HttpResult = struct { const HttpResult = struct {
response_code: u16, // actually 3 digits can fit in u10 response_code: u16, // actually 3 digits can fit in u10
body: []const u8, body: []const u8,
allocator: *std.mem.Allocator,
pub fn deinit(self: HttpResult) void { pub fn deinit(self: HttpResult) void {
self.allocator.free(self.body);
httplog.debug("http result deinit complete", .{}); httplog.debug("http result deinit complete", .{});
return; return;
} }
@ -263,6 +266,14 @@ pub const AwsHttp = struct {
/// HttpResult currently contains the body only. The addition of Headers /// HttpResult currently contains the body only. The addition of Headers
/// and return code would be a relatively minor change /// and return code would be a relatively minor change
pub fn makeRequest(self: Self, endpoint: EndPoint, method: []const u8, path: []const u8, body: []const u8, signing_options: ?SigningOptions) !HttpResult { pub fn makeRequest(self: Self, endpoint: EndPoint, method: []const u8, path: []const u8, body: []const u8, signing_options: ?SigningOptions) !HttpResult {
// Since we're going to pass these into C-land, we need to make sure
// our inputs have sentinals
const method_z = try self.allocator.dupeZ(u8, method);
defer self.allocator.free(method_z);
const path_z = try self.allocator.dupeZ(u8, path);
defer self.allocator.free(path_z);
const body_z = try self.allocator.dupeZ(u8, body);
defer self.allocator.free(body_z);
// TODO: Try to re-encapsulate this // TODO: Try to re-encapsulate this
// var http_request = try createRequest(method, path, body); // var http_request = try createRequest(method, path, body);
@ -270,14 +281,14 @@ pub const AwsHttp = struct {
var http_request = c.aws_http_message_new_request(c_allocator); var http_request = c.aws_http_message_new_request(c_allocator);
defer c.aws_http_message_release(http_request); defer c.aws_http_message_release(http_request);
if (c.aws_http_message_set_request_method(http_request, c.aws_byte_cursor_from_c_str(@ptrCast([*c]const u8, method))) != c.AWS_OP_SUCCESS) if (c.aws_http_message_set_request_method(http_request, c.aws_byte_cursor_from_c_str(@ptrCast([*c]const u8, method_z))) != c.AWS_OP_SUCCESS)
return AwsError.SetRequestMethodError; return AwsError.SetRequestMethodError;
if (c.aws_http_message_set_request_path(http_request, c.aws_byte_cursor_from_c_str(@ptrCast([*c]const u8, path))) != c.AWS_OP_SUCCESS) if (c.aws_http_message_set_request_path(http_request, c.aws_byte_cursor_from_c_str(@ptrCast([*c]const u8, path_z))) != c.AWS_OP_SUCCESS)
return AwsError.SetRequestPathError; return AwsError.SetRequestPathError;
httplog.debug("body length: {d}", .{body.len}); httplog.debug("body length: {d}", .{body.len});
const body_cursor = c.aws_byte_cursor_from_c_str(@ptrCast([*c]const u8, body)); const body_cursor = c.aws_byte_cursor_from_c_str(@ptrCast([*c]const u8, body_z));
const request_body = c.aws_input_stream_new_from_cursor(c_allocator, &body_cursor); const request_body = c.aws_input_stream_new_from_cursor(c_allocator, &body_cursor);
defer c.aws_input_stream_destroy(request_body); defer c.aws_input_stream_destroy(request_body);
if (body.len > 0) { if (body.len > 0) {
@ -290,8 +301,9 @@ pub const AwsHttp = struct {
var context = RequestContext{ var context = RequestContext{
.allocator = self.allocator, .allocator = self.allocator,
}; };
defer context.deinit();
var tls_connection_options: ?*c.aws_tls_connection_options = null; var tls_connection_options: ?*c.aws_tls_connection_options = null;
const host = try std.fmt.allocPrint(self.allocator, "{s}", .{endpoint.host}); const host = try self.allocator.dupeZ(u8, endpoint.host);
defer self.allocator.free(host); defer self.allocator.free(host);
try self.addHeaders(http_request.?, host, body); try self.addHeaders(http_request.?, host, body);
if (std.mem.eql(u8, endpoint.scheme, "https")) { if (std.mem.eql(u8, endpoint.scheme, "https")) {
@ -434,6 +446,7 @@ pub const AwsHttp = struct {
const rc = HttpResult{ const rc = HttpResult{
.response_code = context.response_code.?, .response_code = context.response_code.?,
.body = final_body, .body = final_body,
.allocator = self.allocator,
}; };
return rc; return rc;
} }
@ -535,9 +548,9 @@ pub const AwsHttp = struct {
} }
defer c.aws_signable_destroy(signable); defer c.aws_signable_destroy(signable);
const signing_region = try std.fmt.allocPrint(self.allocator, "{s}", .{options.region}); const signing_region = try std.fmt.allocPrintZ(self.allocator, "{s}", .{options.region});
defer self.allocator.free(signing_region); defer self.allocator.free(signing_region);
const signing_service = try std.fmt.allocPrint(self.allocator, "{s}", .{options.service}); const signing_service = try std.fmt.allocPrintZ(self.allocator, "{s}", .{options.service});
defer self.allocator.free(signing_service); defer self.allocator.free(signing_service);
const temp_signing_config = c.bitfield_workaround_aws_signing_config_aws{ const temp_signing_config = c.bitfield_workaround_aws_signing_config_aws{
.algorithm = .AWS_SIGNING_ALGORITHM_V4, .algorithm = .AWS_SIGNING_ALGORITHM_V4,
@ -647,7 +660,7 @@ pub const AwsHttp = struct {
return AwsError.AddHeaderError; return AwsError.AddHeaderError;
if (body.len > 0) { if (body.len > 0) {
const len = try std.fmt.allocPrint(self.allocator, "{d}", .{body.len}); const len = try std.fmt.allocPrintZ(self.allocator, "{d}", .{body.len});
// This defer seems to work ok, but I'm a bit concerned about why // This defer seems to work ok, but I'm a bit concerned about why
defer self.allocator.free(len); defer self.allocator.free(len);
const content_length_header = c.aws_http_header{ const content_length_header = c.aws_http_header{
@ -833,7 +846,7 @@ fn fullCast(comptime T: type, val: anytype) T {
fn regionSubDomain(allocator: *std.mem.Allocator, service: []const u8, region: []const u8, useDualStack: bool) !EndPoint { fn regionSubDomain(allocator: *std.mem.Allocator, service: []const u8, region: []const u8, useDualStack: bool) !EndPoint {
const environment_override = std.os.getenv("AWS_ENDPOINT_URL"); const environment_override = std.os.getenv("AWS_ENDPOINT_URL");
if (environment_override) |override| { if (environment_override) |override| {
const uri = try std.fmt.allocPrint(allocator, "{s}", .{override}); const uri = try allocator.dupeZ(u8, override);
return endPointFromUri(allocator, uri); return endPointFromUri(allocator, uri);
} }
// Fallback to us-east-1 if global endpoint does not exist. // Fallback to us-east-1 if global endpoint does not exist.
@ -847,7 +860,7 @@ fn regionSubDomain(allocator: *std.mem.Allocator, service: []const u8, region: [
else => "amazonaws.com", else => "amazonaws.com",
}; };
const uri = try std.fmt.allocPrint(allocator, "https://{s}{s}.{s}.{s}", .{ service, dualstack, realregion, domain }); const uri = try std.fmt.allocPrintZ(allocator, "https://{s}{s}.{s}.{s}", .{ service, dualstack, realregion, domain });
const host = uri["https://".len..]; const host = uri["https://".len..];
httplog.debug("host: {s}, scheme: {s}, port: {}", .{ host, "https", 443 }); httplog.debug("host: {s}, scheme: {s}, port: {}", .{ host, "https", 443 });
return EndPoint{ return EndPoint{
@ -921,15 +934,16 @@ const RequestContext = struct {
const Self = @This(); const Self = @This();
pub fn deinit(self: Self) void { pub fn deinit(self: Self) void {
self.allocator.free(self.body); // We're going to leave it to the caller to free the body
// if (self.body) |b| self.allocator.free(b);
if (self.headers) |hs| { if (self.headers) |hs| {
for (hs) |h| { for (hs.items) |h| {
// deallocate the copied values // deallocate the copied values
self.allocator.free(h.name); self.allocator.free(h.name);
self.allocator.free(h.value); self.allocator.free(h.value);
} }
// deallocate the structure itself // deallocate the structure itself
h.deinit(); hs.deinit();
} }
} }
@ -937,11 +951,11 @@ const RequestContext = struct {
var orig_body: []const u8 = ""; var orig_body: []const u8 = "";
if (self.body) |b| { if (self.body) |b| {
orig_body = try self.allocator.dupeZ(u8, b); orig_body = try self.allocator.dupeZ(u8, b);
self.allocator.free(self.body.?); self.allocator.free(b);
self.body = null; self.body = null;
} }
defer self.allocator.free(orig_body); defer self.allocator.free(orig_body);
self.body = try std.fmt.allocPrint(self.allocator, "{s}{s}", .{ orig_body, fragment }); self.body = try std.fmt.allocPrintZ(self.allocator, "{s}{s}", .{ orig_body, fragment });
} }
pub fn addHeader(self: *Self, name: []const u8, value: []const u8) !void { pub fn addHeader(self: *Self, name: []const u8, value: []const u8) !void {

View File

@ -36,7 +36,13 @@ pub fn main() anyerror!void {
const test_json = false; const test_json = false;
if (test_json) try jsonFun(); if (test_json) try jsonFun();
const allocator = std.heap.c_allocator; const c_allocator = std.heap.c_allocator;
var gpa = std.heap.GeneralPurposeAllocator(.{}){
.backing_allocator = c_allocator,
};
defer if (!gpa.deinit()) @panic("memory leak detected");
const allocator = &gpa.allocator;
// const allocator = std.heap.c_allocator;
const options = aws.Options{ const options = aws.Options{
.region = "us-west-2", .region = "us-west-2",