Skip to content

Register task threads before scheduling tasks #8981

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.

Sign up for GitHub

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

Jump to bottom
Merged
duke8253 merged 1 commit into from
Aug 29, 2022
Merged

Register task threads before scheduling tasks #8981

duke8253 merged 1 commit into from
Aug 29, 2022

Conversation

@duke8253
Copy link
Contributor

@duke8253 duke8253 commented Jul 26, 2022

As discussed yesterday in the meeting, this will replace #8708

@duke8253 duke8253 added this to the 10.0.0 milestone Jul 26, 2022
@duke8253 duke8253 self-assigned this Jul 26, 2022
@duke8253
Copy link
Contributor Author

duke8253 commented Jul 26, 2022

[approve ci autest]

@duke8253
Copy link
Contributor Author

duke8253 commented Jul 26, 2022

Hmm, not sure why this is failing the verify plugin tests with Test : Checking that ReturnCode == -2 - Failed , can't recreate it locally either.

@bneradt
Copy link
Contributor

bneradt commented Jul 28, 2022

In case it's helpful, it looks like the verify_global_plugin test fails with a seg fault:

https://ci.trafficserver.apache.org/job/Github_Builds/job/autest/1256/artifact/output/8981/sandbox/verify_global_plugin/_output/0-tr-Default/stream.all.txt

ERROR: verifying a plugin requires a plugin SO file path argument
traffic_server - STACK TRACE: 
/lib64/libpthread.so.0(+0x12ce0)[0x7f5e94630ce0]
traffic_server(_ZNK8APIHooks4headEv+0xc)[0x86fe58]
traffic_server(_ZNK15FeatureAPIHooksI17TSLifecycleHookIDLi9EE3getES0_+0x40)[0x8b0632]
traffic_server(_ZN12AutoStopCont9mainEventEiP5Event+0x2f)[0x8ae5fd]
traffic_server(_ZN12Continuation11handleEventEiPv+0xdd)[0x85f4bf]
traffic_server(_ZN7EThread13process_eventEP5Eventi+0x157)[0xd3b987]
traffic_server(_ZN7EThread15execute_regularEv+0x280)[0xd3bef2]
traffic_server(_ZN7EThread7executeEv+0x107)[0xd3c265]
traffic_server[0xd3aab8]
/lib64/libpthread.so.0(+0x81cf)[0x7f5e946261cf]
/lib64/libc.so.6(clone+0x43)[0x7f5e9375fd83]

@duke8253
Copy link
Contributor Author

duke8253 commented Jul 28, 2022

In case it's helpful, it looks like the verify_global_plugin test fails with a seg fault:

https://ci.trafficserver.apache.org/job/Github_Builds/job/autest/1256/artifact/output/8981/sandbox/verify_global_plugin/_output/0-tr-Default/stream.all.txt

ERROR: verifying a plugin requires a plugin SO file path argument
traffic_server - STACK TRACE: 
/lib64/libpthread.so.0(+0x12ce0)[0x7f5e94630ce0]
traffic_server(_ZNK8APIHooks4headEv+0xc)[0x86fe58]
traffic_server(_ZNK15FeatureAPIHooksI17TSLifecycleHookIDLi9EE3getES0_+0x40)[0x8b0632]
traffic_server(_ZN12AutoStopCont9mainEventEiP5Event+0x2f)[0x8ae5fd]
traffic_server(_ZN12Continuation11handleEventEiPv+0xdd)[0x85f4bf]
traffic_server(_ZN7EThread13process_eventEP5Eventi+0x157)[0xd3b987]
traffic_server(_ZN7EThread15execute_regularEv+0x280)[0xd3bef2]
traffic_server(_ZN7EThread7executeEv+0x107)[0xd3c265]
traffic_server[0xd3aab8]
/lib64/libpthread.so.0(+0x81cf)[0x7f5e946261cf]
/lib64/libc.so.6(clone+0x43)[0x7f5e9375fd83]

Yeah, it's weird that it says verifying a plugin requires a plugin SO file path argument, which sounds like it's missing the so file? Haven't got time to check it yet with the ci docker image, will find sometime next week.

@bryancall
Copy link
Contributor

bryancall commented Aug 1, 2022

[approve ci autest]

@duke8253 duke8253 force-pushed the master-task_thread_start branch from 7d4311d to 0e6fb6e Compare August 1, 2022 23:13
@duke8253
Copy link
Contributor Author

duke8253 commented Aug 2, 2022

[approve ci autest]

@duke8253 duke8253 force-pushed the master-task_thread_start branch from 0e6fb6e to 96606db Compare August 29, 2022 18:56
SolidWallOfCode
SolidWallOfCode reviewed Aug 29, 2022
View reviewed changes
src/traffic_server/traffic_server.cc
std::unique_lock<std::mutex> lock(pluginInitMutex);
plugin_init_done = true;
lock.unlock();
pluginInitCheck.notify_one();
Copy link
Member

@SolidWallOfCode SolidWallOfCode Aug 29, 2022

Choose a reason for hiding this comment

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

Any good reason to not notify all? It might be the case in the future that other things wait on plugin initialization, although really this could be fixed at that point.

Copy link
Contributor Author

@duke8253 duke8253 Aug 29, 2022

Choose a reason for hiding this comment

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

Actually I thought about changing it to notify_all(), but then I thought since we only have one place that's waiting for it, probably should not change it.

@duke8253
Copy link
Contributor Author

duke8253 commented Aug 29, 2022

[approve ci]

@duke8253
Copy link
Contributor Author

duke8253 commented Aug 29, 2022 •
edited
Loading

hmm, I can't seem to get some of the ci builds to start ?

@duke8253
Copy link
Contributor Author

duke8253 commented Aug 29, 2022

[approve ci autest]

@duke8253 duke8253 merged commit 1fdb376 into apache:master Aug 29, 2022
@duke8253 duke8253 mentioned this pull request Aug 29, 2022
Closed
@zwoop
Copy link
Contributor

zwoop commented Jun 29, 2023

Do we need this for 9.2.2 ?

masaori335 added a commit to masaori335/trafficserver that referenced this pull request Jul 24, 2023
@masaori335 @duke8253
register task threads earlier (apache#8981) (apache#585)
f021ba1 
Co-authored-by: Fei Deng <feid@yahooinc.com>
(cherry picked from commit 1fdb376)

Co-authored-by: Fei Deng <duke8253@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@SolidWallOfCode SolidWallOfCode SolidWallOfCode approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@duke8253 duke8253

Labels

Core Event System Threads

Projects

None yet

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@duke8253 @bneradt @bryancall @zwoop @SolidWallOfCode