Skip to content

fix the program cannot be terminated through ctrl-c#2

Merged
jschwinger233 merged 1 commit intojschwinger233:mainfrom
mozillazg:fix-cancel
Mar 12, 2023
Merged

fix the program cannot be terminated through ctrl-c#2
jschwinger233 merged 1 commit intojschwinger233:mainfrom
mozillazg:fix-cancel

Conversation

@mozillazg
Copy link
Contributor

No description provided.

@jschwinger233
Copy link
Owner

Thanks for your PR.

Basically I have no question in this change, just want to add a point:

I'm not sure how this change can fix the non-interrupting issue. If ctrl-c doesn't stop the process, there must be somewhere blocking for a long term so that process has no change to reach the "cancel point" to check <-ctx.Done(). As far as I can see, this can only happen on the bpf syscall LookupAndDelete or fs IO skbw.Write and pcapw.WritePacket, but none of these could be fixed by this PR, could it?

@mozillazg
Copy link
Contributor Author

If ctrl-c doesn't stop the process, there must be somewhere blocking for a long term so that process has no change to reach the "cancel point" to check <-ctx.Done()

skbdump/main.go

Lines 109 to 119 in 105f01e

for {
if err := bpfObjs.DataQueue.LookupAndDelete(nil, &data); err == nil {
break
}
select {
case <-ctx.Done():
return
case <-time.After(time.Microsecond):
continue
}
}

In most cases, line 111 will always be executed and there is no way to break the first for loop. for example:

skbdump -i eth0

I can not terminate this process through ctrl-c.

@jschwinger233
Copy link
Owner

Understood, you pointed out that the process can't reach ctx check point when two bpf syscalls keep succeeding fetching events.

You're correct.

@jschwinger233 jschwinger233 self-requested a review March 12, 2023 04:10
@jschwinger233 jschwinger233 merged commit 79533b0 into jschwinger233:main Mar 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants