fix(container): Propagate engine initialization errors to the caller #1020
Conversation
rabbitstack
force-pushed
the
container-plugin-engine-initialization-error-handling
branch
from
e56b8e9 to
c7c0968
Compare
Rules files suggestions |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rabbitstack The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rabbitstack The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
If the specific container engine worker fails during initialization, the error is silently skipped making it hard to troubleshoot the real problem. i Instead, accumulate and bubble up all the errors to the async handler. Signed-off-by: rabbitstack <[email protected]>
rabbitstack
force-pushed
the
container-plugin-engine-initialization-error-handling
branch
from
46c632c to
76943d2
Compare
Rules files suggestions |
| errmsg := C.CString("") | ||
| ptr := StartWorker((*[0]byte)(C.echo_cb), cstr, &enabledSocks, &errmsg) | ||
| if ptr == nil { | ||
| fmt.Println("Failed to start worker; nothing configured?") | ||
| fmt.Println(fmt.Sprintf("Failed to start worker; nothing configured? %s", C.GoString(errmsg))) | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
This snippet introduces a memory leak, because C.Cstring("") allocates a C string on the heap, and the Go runtime will not garbage-collect it. We should add a call to defer C.free(...). but in order to be sure that this is called just after StartWorker(...) invocation, and regardless the fact that this function could panic, I would add a new wrapping function like the following:
func startWorker(...) unsafe.Pointer {
errmsg := C.CString("")
defer C.free(unsafe.Pointer(errmsg))
return StartWorker((*[0]byte)(C.echo_cb), cstr, &enabledSocks, &errmsg)
}3 participants
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area plugins
What this PR does / why we need it:
Which issue(s) this PR fixes:
If the specific container engine worker fails during initialization, the error is silently skipped, making it hard to troubleshoot the real problem. Instead, accumulate and bubble up all the errors to the async handler.
Special notes for your reviewer: