Skip to content

Fix data race when create POSIX thread#3942

Closed
gelldur wants to merge 1 commit intopocoproject:masterfrom
gelldur:master
Closed

Fix data race when create POSIX thread#3942
gelldur wants to merge 1 commit intopocoproject:masterfrom
gelldur:master

Conversation

@gelldur
Copy link
Copy Markdown

@gelldur gelldur commented Feb 11, 2023 •
edited
Loading

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()

@gelldur
Fix data race when create POSIX thread
9c8a79b 
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 gelldur mentioned this pull request Feb 11, 2023
Closed
@gelldur
Copy link
Copy Markdown
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
Copy Markdown
Member

aleks-f commented Mar 17, 2023

@gelldur please push a dummy commit to trigger CI

@aleks-f
Copy link
Copy Markdown
Member

aleks-f commented Mar 22, 2023 •
edited
Loading

redesigned for 1.13

@aleks-f aleks-f closed this Mar 22, 2023
aleks-f added a commit that referenced this pull request Mar 22, 2023
@aleks-f
fix(Thread): Fix data race when create POSIX thread #3942
35982ac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@aleks-f aleks-f

Labels

bug

Projects

None yet

Milestone

Release 1.12.5

Development

Successfully merging this pull request may close these issues.

2 participants

@gelldur @aleks-f