padgettr / rfm

Simple file manager for dwm
Other
11 stars 2 forks source link

how do you sync the threads? #3

Closed guyuming76 closed 1 year ago

guyuming76 commented 1 year ago

when i read into your code, i don't quite understand your logic to sync the threads. i did not find any locking in your code.

for example, in the following code, what if just after line 1032 executed, line 228 execute in another thread, and then line 1033?

https://github.com/padgettr/rfm/blob/76fed73021058aad0d4c184cd94a9483be940734/rfm.c#L1032

padgettr commented 1 year ago

I don't think it would be a problem here, line 228 would only execute if there was a problem with the thread and it didn't stop in the timeout. In that case it would be stuck on line 1019, which is why I added the extra thread to deal with bad hardware or slow networks. If the thread won't stop we abandon it by incrementing the threadID, so it is very unlikely that line 1033 would ever run. But I agree there is a potential race condition there if line 1019 or line 1022 unblocked after the time out and before threadID is implemented. The chance of it being triggered is very small I think? If it was triggered wouldn't thread running just get set to false as intended by both anyway?

Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: guyuming @.> Sent: Monday, October 10, 2022 12:23:29 PM To: padgettr/rfm @.> Cc: Subscribed @.***> Subject: [padgettr/rfm] how do you sync the threads? (Issue #3)

when i read into your code, i don't quite understand your logic to sync the threads. i did not find any locking in your code.

for example, in the following code, what if just after line 1032 executed, line 228 execute in another thread, and then line 1033?

https://github.com/padgettr/rfm/blob/76fed73021058aad0d4c184cd94a9483be940734/rfm.c#L1032

— Reply to this email directly, view it on GitHubhttps://github.com/padgettr/rfm/issues/3, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AB25FQY7JPD2JTQR4W4LOSTWCP4DDANCNFSM6AAAAAARBIAQ2E. You are receiving this because you are subscribed to this thread.Message ID: @.***>

guyuming76 commented 1 year ago

And here, after thread created for readDir, rfm_readDirThreadRunning set to true, is it possible that OS schedule failed to schedule the "set to true statement" in time? https://github.com/padgettr/rfm/blob/76fed73021058aad0d4c184cd94a9483be940734/rfm.c#L1135

and here :https://github.com/padgettr/rfm/blob/76fed73021058aad0d4c184cd94a9483be940734/rfm.c#L1067

If the flag is not set to true in time, will this be race condition?

for this particular issue, would it be better if we move line 1135 into readDir function?

guyuming76 commented 1 year ago

If it was triggered wouldn't thread running just get set to false as intended by both anyway?

I don't think so, in the if statement, dirReadThreadID has not changed yet, so we will set thread running to false. But we actually set it to false, dirReadThreadID may have already changed, which means we will set it to false when we shouldn't, the status shall be running under new thread id. OS scheduler is not predictable, right? i cannot say in which statement we will wait for a long time for the next chance to run.

guyuming76 commented 1 year ago

i think your logic is: fill rfm_FileAttributeList in readDir; consume rfm_FileAttributeList in UpdateIconView; fill_store orchestrates all these.

but the problem is, in UpdateIconView, rfm_readDirThreadRunning status is only checked at the beginning. what if readDir runs in the middle of UpdateIconView?

And since the goodness of rfm is its short and simple, why not just put readDir and UpdateIconView in the same thread and run sequentially?

i had written something about in memory data refresh in .net https://social.msdn.microsoft.com/Forums/office/en-US/46395303-9bf9-4a15-85b5-16e193abdfe2/large-object-memory-cache-refreshing?forum=sharepointdevelopment . And i am thinking of migrating it to linux and C (i don't know whether anything else existing can do the same), but it takes effort.

padgettr commented 1 year ago

Update iconview can't be run in the same thread as gtk3 is not thread safe. I tried that and got lots of problems. The code attempts to do all updating of gtk in the main thread only. If readdir runs at the same time as update iconview, then dirreadthreadID must have been incremented in stop all threads via rfm_stop_all because this all runs in the main thread and has to run before update. This would result in readDir dumping the data, because the threadID is old. However on a closer look I see that there is a small chance that the file attributes could be updated in an abandoned thread if increment ID happens at the same time the old thread is checking ID, which would result in a race condition between the main thread clearing the list and the old readdir thread adding an item, so possibly wrong item is added in the view since attribute list is global. Yuk! My brain hurts. I think I want to go back to single thread...

Initially it was only a single thread application, as I'll admit I don't normally do any multithread coding, and it was a hard desicion as I wanted to keep it simple. I only recently added the threading because I got sick of waiting for slow network drives at work! Thats the problem, I use this every day at work and the code gets hacked about as I need new stuff but I never seem to get time to tidy it all up! But maybe you are right and it should go back to a single thread application. It was definitly much more simple and easy to understand! The thumbnail thread should be robust, as they are updated by the thumbnail being written to disk through inotify - it used to run independently as a forked process.

Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: guyuming @.> Sent: Tuesday, 11 October 2022, 06:32 To: padgettr/rfm @.> Cc: Dr Rodney Padgett @.>; Comment @.> Subject: Re: [padgettr/rfm] how do you sync the threads? (Issue #3)

i think your logic is: fill rfm_FileAttributeList in readDir; consume rfm_FileAttributeList in UpdateIconView.

but the problem is, in UpdateIconView, rfm_readDirThreadRunning status is only checked at the beginning. what if readDir runs in the middle of UpdateIconView?

And since the goodness of rfm is its short and simple, why not just put readDir and UpdateIconView in the same thread and run sequentially?

i had written something about in memory data refresh in .net https://social.msdn.microsoft.com/Forums/office/en-US/46395303-9bf9-4a15-85b5-16e193abdfe2/large-object-memory-cache-refreshing?forum=sharepointdevelopment . And i am thinking of migrating it to linux and C, but it takes effort.

— Reply to this email directly, view it on GitHubhttps://github.com/padgettr/rfm/issues/3#issuecomment-1274108980, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AB25FQ66FBT42BKHOIESI73WCT3X5ANCNFSM6AAAAAARBIAQ2E. You are receiving this because you commented.Message ID: @.***>