|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/5 18:54
#21 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@Capehill
Quote:
Trying this now, will first try native-threading-patch fix with "mx->u.i.acquired++" before return 0. ps. can show an example with that "add try-catch around "join" call.", never used this (and mostly never use c++ for real:) ) Quote:
That one works correctly, throw at me in the shell: Quote:
|
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/5 19:02
#22 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2007/7/14 21:30 From Lothric
Posts: 1203
|
@kas1e
Try-catch example: https://github.com/minetest/minetest/b ... threading/thread.cpp#L120 I cannot say what should be done in the catch branch though: should waiting fail or carry on with thread deletion? Print some error for starters... |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/5 19:51
#23 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@Capehill
Ah, you mean that a usual try and catch (was by some stupid reason think that this is something called exactly try-catch :) ). Anyway, tried firstly as you say that in thread-native-patch:
int
Now, that what we have in output when run testunits for this https://github.com/minetest/minetest/b ... ittest/test_threading.cpp Quote:
So, it does 5 joins. And seems passed the testStartStopWait(). But then, when it starts to test testAtomicSemaphoreThread() from above-mentioned test_threading.cpp, then it seems to fail again. Question is: do we deal with first issue now, and meeting with another one in semaphores ? For testStartStopWait test i can see in void TestThreading::testStartStopWait() : for (size_t i = 0; i != 5; i++) { ... } , so maybe indeed correctly works now and we need to approve patch to adtools ? But then second question, why it stops on second joing from void TestThreading::testAtomicSemaphoreThread():
void TestThreading::testAtomicSemaphoreThread()
Does that maybe issue in our semaphores-POSIX implementation? Frederik, maybe you have an idea about the second one, and if we correctly fix now gthread_mutex_trylock() and this one also needs to be added to adtools ? PS. Was in hope that this fix in thread-native will fix that issue: https://github.com/sba1/adtools/issues/82, but nope, still something else (but also probably close enough to what you find). But our new issue also looks pretty close to that bug report too. Maybe will be just another bug in the native-threading-patch. PS2. Oh, and what is interesting, the actual game can't start because of the same issue :) I.e. when I click "play game" it shows me in the shell the same "before join, after join, before join" and hang. Just like when I run test_threading and it acts when checking testAtomicSemaphoreThread() :) At least now I know why the game didn't start (were about to start debugging network code) Edited by kas1e on 2021/1/5 20:23:27
Edited by kas1e on 2021/1/5 20:30:59 Edited by kas1e on 2021/1/5 20:56:34 Edited by kas1e on 2021/1/5 21:01:31 |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/6 8:27
#24 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2007/7/14 21:30 From Lothric
Posts: 1203
|
@kas1e @salass00
I browsed through the thread patch and it seems like a potential problem that threadstore is accessed without semaphore in join, detach and cond_wait, at least. find_threadentry_by_id: https://github.com/sba1/adtools/blob/m ... s-thread-model.patch#L942 __gthread_join: https://github.com/sba1/adtools/blob/m ... s-thread-model.patch#L963 Also there are some FIXMEs left: https://github.com/sba1/adtools/blob/m ... s-thread-model.patch#L966 |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/6 8:30
#25 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@Capehill
Quote:
Maybe can cook something to test with? I mean is it just a few additional lines that need to be added in those functions for a test, or need some rewriting? Anyway, I put bunch of printfs to the testAtomicSemaphoreThread(), like this:
void TestThreading::testAtomicSemaphoreThread()
And that what I have in the output:
we in the testAtomicSemaphoreThread
So first "For" for 4 threads works ok (created 4 AtomicTestThreads and start them ), then trigger.post(num_threads) also seems ok, and then on the second "For" for 4 threads, it passes the only the first one, and hang on the second thread while doing thread->wait() in the join(). Edited by kas1e on 2021/1/6 8:56:09
Edited by kas1e on 2021/1/6 9:08:33 Edited by kas1e on 2021/1/6 9:12:57 |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/6 13:11
#26 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2007/7/14 21:30 From Lothric
Posts: 1203
|
@kas1e
Threadstore protection is a bit complex and would be more than 2 lines of code (missing in various places). In fact I'm not familiar with this code at all so it would take some more time to investigate. Could you try to add serial debug in __gthread_join near line 966:
+ /* FIXME: This is very ugly, use SIGF_SINGLE */
|
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/6 15:47
#27 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@Capehill
Added your printf, and have on serial when things hangs never ended loop of our printfs: Quote:
It just never ends, and it waits forever and so because of that we hang. So yeah, seems your suggestion is the correct one. Or at least we close enough :) As i see from SBA's comment at top of that "Fix me" code, he think about use of SIGF_SINGLE , is it of any help in right direction ? Edited by kas1e on 2021/1/6 16:14:28
Edited by kas1e on 2021/1/6 16:18:11 |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/6 20:35
#28 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2006/11/30 11:30 From Finland
Posts: 1775
|
@kas1e
The finished field in struct threadentry may have to be made volatile. Otherwise the compiler will probably read it only once at the beginning of the loop and then assume that it will never change. |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/6 20:41
#29 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@Salas00
You mean that one : "struct threadentry *next_in_cond_wait_list; " ? In meantime, will donate you and Capehill a bit for all the help you do with it. EDIT: done! |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/6 20:54
#30 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2006/11/30 11:30 From Finland
Posts: 1775
|
@kas1e
No, the one line 416 (try changing "int finished;" to "volatile int finished;"): https://github.com/sba1/adtools/blob/m ... s-thread-model.patch#L416 |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/6 21:01
#31 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@salas00
Quote:
Nope, didn't help. Still bring those "Waiting for thread" from our printf in __gthread_join |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/7 8:26
#32 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2007/7/14 21:30 From Lothric
Posts: 1203
|
@kas1e
You could put more debug prints near code where finished flag is set, like:
+static int __gthread_entry(STRPTR args UNUSED, int32 length UNUSED, APTR execbase UNUSED)
PS. Thanks for the donation! |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/7 9:30
#33 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@Capehill
Quote:
As I can see, the first 4 thread waiting are fine (so our first 4 for() from the test case). Then, when our second test from the test case start (that atomicsemaprhore one), then 1 thread waiting fine, but then on second one when we have a halt, it does 3 additional "findtask/thr=" and seems this "while" with SIGBREAKFs didn't find a shit, and all going to loop "waiting for the thread"? It's like the second test also does 4 times the test as expected, but seems to wait for answers borks for 2,3, and 4 threads. EDIT: I also comment out for the sake of more clean output from test-threading code a TEST(testStartStopWait) and keep only our failing one: TEST(testAtomicSemaphoreThread), and clean output with our new debugs are: Quote:
Edited by kas1e on 2021/1/7 10:25:53
|
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/7 10:04
#34 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2006/11/30 11:30 From Finland
Posts: 1775
|
@kas1e
Try adding some debugs around the ObtainSemaphore() call in __gthread_entry(): https://github.com/sba1/adtools/blob/m ... s-thread-model.patch#L917 If the CTRL-F signal doesn't get sent it might be because the semaphore is being held by another process. |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/7 10:20
#35 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@Salas00
Will check now, but the line you point out are from __gthread_create(), so i can add printfs in both __gthread_create() around ObtainSemaphore and in __gthread_entry() also around ObtainSemaphore |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/7 10:23
#36 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@Salas00
Done, result: Quote:
As I can see it only 1-time call ObtainSemaphore from the __gthread_entry at the end. Probably that why we have 1 thread from debug output passes, but the other 3 never happens, and waiting forever for a 3 threads to answer. At least how I understand the output. It also looks like we send it 4 times CTRL_F, but only one is received (right?) Interesting also, that at first, we have clean ObtainSemaphore() call from __gthread_create, without anything else, but then all next ones mixed with CTRL+F checks, but dunno maybe that expected to be like this. Edited by kas1e on 2021/1/7 10:42:44
|
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/7 12:53
#37 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2006/11/30 11:30 From Finland
Posts: 1775
|
@kas1e
Unless I've misunderstood your debug output then all four threads have received the CTRL-F signal. The problem seems to be that only one of them manages to return from "thr->result = thr->entry(thr->args);" as there is only one message "Before ObtainSemaphore in __gthread_entry" in output. Edit: No I think the problem is that sem_post() (trigger.post() in TestThreading::testAtomicSemaphoreThread()) only successfully triggers one of the threads. Could you maybe add some debug output around trigger.wait() to check this? https://github.com/minetest/minetest/b ... t/test_threading.cpp#L125 |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/7 12:54
#38 |
---|---|---|
Home away from home
![]() ![]() Joined:
2007/9/11 12:31 From Russia
Posts: 6707
|
@Salas00
Quote:
Do you mean with atomic type in the native-thread-patch ? |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/7 13:12
#39 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2006/11/30 11:30 From Finland
Posts: 1775
|
@kas1e
Can you check if there is a problem with the trigger.wait()/trigger.post() mechanism (see my edit above)? |
|
|
Re: MineCraft (MineTest) work in progress help need it |
Posted on: 1/7 13:32
#40 |
---|---|---|
Just can't stay away
![]() ![]() Joined:
2006/11/30 11:30 From Finland
Posts: 1775
|
@kas1e
I made some changes in the sem_wait()/sem_timedwait() and sem_post() functions that should hopefully fix the problem. In the previous version it was possible that when sem_post() was called four times in quick succession that only the first thread in the wait list was signaled four times and the others were just left waiting. https://www.dropbox.com/s/43b4g16kvhd7 ... emaphore-amigaos4.7z?dl=0 |
|