Skip to content

core(TLS): force TlsAbstraction initialization before main()#20802

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
alalek:core_tls_init_TlsAbstraction
Oct 6, 2021
Merged

core(TLS): force TlsAbstraction initialization before main()#20802
opencv-pushbot merged 1 commit intoopencv:3.4from
alalek:core_tls_init_TlsAbstraction

Conversation

@alalek
Copy link
Copy Markdown
Member

@alalek alalek commented Oct 4, 2021

relates #20606

@alalek alalek mentioned this pull request Oct 4, 2021
3 tasks
#endif // CV_USE_FLS
#endif // _WIN32

static TlsAbstraction* g_force_initialization_of_TlsAbstraction = getTlsAbstraction();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do this rather than fix the startup threading of getTlsAbstraction() If someone is going to code a fix for this issue, risk code change, and the testing...I suggest a better use of effort is to fix the actual start issue rather than a partial workaround for the startup issue.

And this code change only assists with making that function call before main(). It doesn't help with other static calls, lambdas, Windows DLLs and DllMain(), probably Linux shared libraries, etc.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Which fixes do you want to add?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would not add this code into the opencv project. The specific user that reported a specific problem in their specific project due to a specific race-condition should use this kind of code in their project. Not OpenCV itself.
https://en.cppreference.com/w/cpp/language/siof

This hack (which it seems you have already merged) doesn't work across compile units. This hack create yet another race condition. The race condition that existed before continues to exist. And now this hack creates yet another.

A reliable "fix" requires rethinking the TLS startup (and shutdown) behavior. The startup can likely be fixed in C++11. The shutdown is very difficult (maybe impossible) to fix without a "shutdown manager" which requires classes/objects requiring shutdown handling to formall register themselves with the shutdown manager.

@alalek
Copy link
Copy Markdown
Member Author

alalek commented Oct 6, 2021

👍

@opencv-pushbot opencv-pushbot merged commit e585192 into opencv:3.4 Oct 6, 2021
@alalek alalek mentioned this pull request Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants