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

Fix data race when create POSIX thread #3942

Closed
wants to merge 1 commit into from

Conversation

gelldur
Copy link

@gelldur gelldur commented Feb 11, 2023

When creating thread using pthread_create() _pData->thread will be set. It could lead to data race as in runnableEntry() we refer to that variable.

Instead use pthread_self().
getName() is already under mutex.

Example stack trace:

* thread #1, name = 'App', stop reason = signal SIGSEGV
    frame #0: 0x00007ff6c3ea5722 ld-musl-x86_64.so.1`pthread_setname_np + 98
    frame #1: 0x0000558a8d8120ac App`Poco::ThreadImpl::runnableEntry(void*) [inlined] (anonymous namespace)::setThreadName(thread=0, threadName="aPlAn_2[#2]") at Thread_POSIX.cpp:71:6
  * frame #2: 0x0000558a8d81209f App`Poco::ThreadImpl::runnableEntry(pThread=0x00007ff6c2f32830) at Thread_POSIX.cpp:354:2

Issue only occurs sometimes as setThreadName is called with null thread. I believe this is data race.

In Thread_POSIX.cpp

		FastMutex::ScopedLock l(_pData->mutex);
		_pData->pRunnableTarget = pTarget;
		if (pthread_create(&_pData->thread, &attributes, runnableEntry, this))
		{
			_pData->pRunnableTarget = 0;
			pthread_attr_destroy(&attributes);
			throw SystemException("cannot start thread");
		}

We call pthread_create which should set _pData->thread,
But in ThreadImpl::runnableEntry we already expect is set.

setThreadName(pThreadImpl->_pData->thread, reinterpret_cast<Thread*>(pThread)->getName());

I belive that replacting pThreadImpl->_pData->thread with pthread_self() can help. Other way is to set thread name under mutex but it could conflict/deadlock with getName() as is already under mutex.
Way other solution is to set thread name after pthread_create but again we would have deadlock with getName()

When creating thread using pthread_create() `_pData->thread` will be set.
It could lead to data race as in runnableEntry() we refer to that variable.

Instead use pthread_self().
getName() is already under mutex.
@gelldur
Copy link
Author

gelldur commented Feb 24, 2023

After tests on production it seems to solve issue.

@aleks-f aleks-f self-assigned this Mar 17, 2023
@aleks-f aleks-f added the bug label Mar 17, 2023
@aleks-f
Copy link
Member

aleks-f commented Mar 17, 2023

@gelldur please push a dummy commit to trigger CI

@aleks-f
Copy link
Member

aleks-f commented Mar 22, 2023

redesigned for 1.13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants