Bug 391856 - FileManagerListJob::m_listQueue : needs mutex protection?
Summary: FileManagerListJob::m_listQueue : needs mutex protection?
Status: RESOLVED NOT A BUG
Alias: None
Product: kdevplatform
Classification: Developer tools
Component: project (other bugs)
Version First Reported In: git master
Platform: Compiled Sources All
: NOR task
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-14 15:42 UTC by RJVB
Modified: 2018-03-14 22:37 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description RJVB 2018-03-14 15:42:17 UTC
Hi,

I notice that the QQueue<> m_listQueue in the FileManagerListJob class is populated and possibly pruned from the main thread while being emptied from inside the job itself.

I think that can lead to race conditions (cf. #355241?). Shouldn't access to this FIFO be protected with a mutex to prevent such situations, however unlikely they might be?
Comment 1 Milian Wolff 2018-03-14 17:41:17 UTC
where is it accessed from a background thread? that should not happen. the only background-thread activity to my knowledge happens within the QtConcurrent::run section, which then dispatches its results through QMetaObject::invokeMethod, which will get handled by the main thread again.
Comment 2 RJVB 2018-03-14 18:35:27 UTC
Ahem, I should have 2nd-guessed myself and dump thread IDs from within FMLJ methods :(

I always thought the list job itself runs on a background thread and thus that FileManagerListJob::startNextJob() is called on that thread when the class sends itself the nextJob signal. It inherits KJob after all, and is controlled via asynchronous methods (if that's the correct term).

So exactly what is running on a background thread here, or should I say, why does the class inherit KJob if the only background activity is in the QtConcurrent::run() call?

(BTW, the invokeMethod() call is made after the call into QtConcurrent; I agree it targets the main thread.)
Comment 3 Milian Wolff 2018-03-14 19:28:47 UTC
parallelism doesn't necessarily require threads, KJob is event driven.

And yes, the invokeMethod isn't done after the QtConcurrent::run, but more importantly, it's done from _within_ the functor run in the background.

Anyhow, closing this report, it's clearly invalid.
Comment 4 RJVB 2018-03-14 22:37:20 UTC
Well, my bad, sorry for the noise.