Skip to content
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.

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

threads: helper function TmThreadsWaitForUnpause #11923

Closed
wants to merge 1 commit into from

Conversation

jasonish
Copy link
Member

@jasonish jasonish commented Oct 9, 2024

The pattern of checking the pause flag, setting to paused then
waiting to unpause was done enough times to factor out into its own
function. This is also needed by library users who bring their own
packet acquisition threads.

The pattern of checking the pause flag, setting to paused then
waiting to unpause was done enough times to factor out into its own
function. This is also needed by library users who bring their own
packet acquisition threads.
Comment on lines +361 to +368
void TmThreadsWaitForUnpause(ThreadVars *tv)
{
if (TmThreadsCheckFlag(tv, THV_PAUSE)) {
TmThreadsSetFlag(tv, THV_PAUSED);
TmThreadTestThreadUnPaused(tv);
TmThreadsUnsetFlag(tv, THV_PAUSED);
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is now the only user of TmThreadTestThreadUnPaused, perhaps they should just be merged and this could be the new TmThreadTestThreadUnPaused - however, I feel the "Wait" in the name is more descriptive than Test, as TmThreadTestThreadUnPaused does wait as well.

Copy link

codecov bot commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.35%. Comparing base (d5dd549) to head (b47809b).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11923      +/-   ##
==========================================
- Coverage   82.70%   78.35%   -4.36%     
==========================================
  Files         912      912              
  Lines      249102   248905     -197     
==========================================
- Hits       206018   195019   -10999     
- Misses      43084    53886   +10802     
Flag Coverage Δ
fuzzcorpus 60.71% <0.00%> (+0.03%) ⬆️
livemode 18.71% <90.90%> (-0.01%) ⬇️
pcap ?
suricata-verify ?
unittests 59.00% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 23072

TmThreadTestThreadUnPaused(tv);
TmThreadsUnsetFlag(tv, THV_PAUSED);
}
TmThreadsWaitForUnpause(tv);
Copy link
Member

@victorjulien victorjulien Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kind of wondering if we even pause a thread... other than it starting paused? If not, this could move out of the while loop? I know this stuff used to be a lot more complex in the past due to ideas about thread restarts and such.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I think I looked at they only start paused. So this is only done for some startup synchronization and never again, so things can be simplified further here I think.

@jasonish
Copy link
Member Author

Replaced by #11943.

@jasonish jasonish closed this Oct 11, 2024
@jasonish jasonish deleted the threads-wait-for-unpause/v1 branch October 13, 2024 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants