@georg So, because of that the same value of tc_UserData, i still Null exactly that tc_UserData and crash gone !!
Through i do not know where that tc_UserData is used as well, as in BTThreadingWTF.cpp i change that as i show before, but still NULLed it at begining deal with crash , maybe somewhere else it in use, dunno.
On exit, tc_UserData still have that value from odyssey, but with FindTask(NULL)->tc_UserData = NULL; at main there no crash anymore.
I even do run it 10 times in row, and valut in tc_UserData always the same (i was expected it will be changed from run to run, after we NULL it)
On exit, tc_UserData still have that value from odyssey, but with FindTask(NULL)->tc_UserData = NULL; at main there no crash anymore.
Instead of setting it to NULL at the start of main() you should do it at the end. It's the same for Odyssey, but other software might have the same problem if you don't set it to NULL. Or even better at the start and at the end, in case someone runs another program in a shell which doesn't clear it on exit before Odyssey.
And of course you should fix the actual bug in Odyssey as well.
So, OS4 is incompatible with AmigaOS 3.x? Many programs use tc_UserData. tc_UserData is meant to be *used* by the programs, not by the operating system (hence the name).
And of course you should fix the actual bug in Odyssey as well.
If only i found where is it :)
@georg Only now got what you mean, you mean exactly BCThreadSpecificWTF.cpp (i just forgot about that one at all), and there i didn't change anything, its still the same tc_UserData everywhere (in threadSpecificKeyCreate() and in threadSpecificKeyDelete() ).
So, OS4 is incompatible with AmigaOS 3.x? Many programs use tc_UserData. tc_UserData is meant to be *used* by the programs, not by the operating system (hence the name).
It's not a bug in AmigaOS but in Odyssey (maybe in Roman's parts only), it obviously crashes if tc_UserData wasn't NULL when it's started.
tc_UserData is indeed for user use, but you can't assume any value before you use it if you running on someone else's process (shells in this case but could be another program running Odyssey direct) or after you run some external program on your process (not relevant here).
To be most "friendly" you might save and restore the start value if running in someone else's process, but then again they shouldn't assume you did that.
No idea what it is, but "MUI imagespace screen notify" child process is still running after exit, and it's still waiting for some signals which shouldn't happen either.
That is a process created by MUI which will flush all unused images used on a just closed screen. This process is started with NP_Child=FALSE.
and NEWLIST(...) is bullshit, not needed, when using AllocSysObject(...).
so *key should be a (struct *Node) type, there for malloc() is wrong command to allocate the memory, because malloc() was private memory, and t->tc_UserData comes from outside the process.
I have a problem to understand way the List is allocated here at all, should this not be provided when thread starts, after the thread is closed the (struct *Task) will be deleted so getting or poking at Task -> tc_UserData after the thread has stopped, is like asking for trouble, hopefully this not what is happening.
t->tc_UserData = (struct List *) IExec -> AllocSysObject(ASOT_LIST, TAG_END);
No, it should be t->tc_UserData = IExec -> AllocSysObject(ASOT_LIST, ASOTLIST_Min, TRUE, ASO_NoTrack, FALSE, TAG_END); (struct MinList *) or (struct List *) casting for setting tc_UserData makes no sense. It should be a MinList instead of a List to be compatible to the rest of the MorphOS code, and enable resource tracking because it's probably never freed again, if it would be freed somewhere not setting tc_UserData to NULL there would be very strange.
Quote:
so *key should be a (struct *Node) type,
It has to be a MinNode, if the struct ThreadSpecificNode in the comment is the one which is actually used, allocated with IExce -> AllocSysObject(ASOT_NODE, ASONODE_Min, TRUE, ASONODE_Size, sizeof(struct ThreadSpecificNode), TAG_END);
In case it's not guaranteed that all ThreadSpecificNode are freed additionally with ASO_NoTrack, FALSE.
Quote:
there for malloc() is wrong command to allocate the memory, because malloc() was private memory, and t->tc_UserData comes from outside the process.
Not in threadSpecificKeyCreate() and threadSpecificKeyDelete(), both use t = FindTask(NULL) and malloc()/MEMF_PRIVATE could be used. For using IExec->AllocSysObject() with ASO_NoTrack, FALSE it's required that allocating and freeing is done in the same process as well or the destructors would be called when a different process ends.
Even, and if, its not code have bugs, its aos4 adaptation to already working on mos code need it.
Quote:
and NEWLIST(...) is bullshit, not needed, when using AllocSysObject(...).
You says it like you don't know that its code done for morphos, and there is no AllocSysObject(...) at all in that code and never was (and why it should be here, if i never-ever touch that file). Sayint that NEWLIST(...) is "bullshit" here because AllocSysObject(...) in use its a bit of strange. Code done for morphos firstly to works on, of course there is everything done without meaning of os4 and that file in question wasn't touched at all, and ther is no AllocSysObject(...) was. Omiga!
Its like saying on code which done for morphos and have pure a(), its "Bullshit" because didn't use IFace->b(), because IFace already in use (but it didn't, as it done for mos).
@joerg Will check soon if 1.16 port have the same non-null tc_UserData on exit from or not (1.16 didn't crashes like this), and if it something recent added, then it just there need some adaptation for os4.
Also will check morphos version to see how tc_UserData looks like for them on exit, just to be sure if there differences between our port and mos original (and where, in odyssey's code or in OS).
@Andy Quote:
If that function is invoked from the main process, it will corrupt stuff id tc_UserData is not NULL at program start.
I assume nothing corrupts on morphos and all code is fine (for mos), i will check today how tc_UserData looks like after quit on mos, so can know if it odyssey's code need adapt for os4, or OS differences
Edited by kas1e on 2014/3/1 6:33:50 Edited by kas1e on 2014/3/1 6:35:26 Edited by kas1e on 2014/3/1 6:35:49 Edited by kas1e on 2014/3/1 6:48:01
@all tc_UserData in 1.16 os4 port are 0x0000000 all the time (on run/exit/run) and i just to grep on tc_UserData in all 1.16 sources and didn't find them (as well as no threadSpecificKeyCreate() or threadSpecificKeyDelete() ), so its all fresh stuff happens after 1.16.
That mean no surprise it didn't crashes with 1.9 and 1.16.
Also checked morphos 1.23 version on morphos : before run tc_UserData are NULL, after running tc_UserData have some value and on exit, tc_UserData are NULL as well, and not value it have on running => What mean, on morphos that original code fine enough for morphos, and only os4 fixes need it. Of course i can null at beginning and end, but imho better to fix code to make it works ok on os4.
Regrep for tc_UserData in all 1.23 sources, and they are only in:
BCSharedTimerMorphos.cpp, at begining of TimerFunc():
Even, and if, its not code have bugs, its aos4 adaptation to already working on mos code need it.
Are you using MorphOS or are you using AmigaOS4?
Cleary malloc() is Shared on MorphOS, while malloc() is private on AmigaOS4. So you can NOT use the same code.
Quote:
You says it like you don't know that its code done for morphos, and there is no AllocSysObject(...) at all in that code and never was (and why it should be here, if i never-ever touch that file). Sayint that NEWLIST(...) is "bullshit" here because AllocSysObject(...) in use its a bit of strange.
Yes its code written for MorphOS, and it need to be code written for AmigaOS4, you need to change what needs to be changed, what works on MorphOS does not mean it works on AmigaOS.
Like I say its not needed when using the AllocSysObject(...), it does it, so your just duplicating code. NEWLIST was used in the stone age, when people where using AmigaOS3.x.
Joerg correct me on the AllocSysObject stuff, this is what you should be using, not malloc, its wrong, because malloc is newlib stuff not AmigaOS stuff, the correct memory allocation routine for AmigaOS is AllocVec() and AllocMem(), you should never use anything else reserving memory for things to bused for Amiga Libraries routines, and more correct way to do thins is AllocSysObject()/AllocSysObjectTags() and AllocDosObject()/AllocDosObjectTags() when you allocate system structures like Struct Node, Struct List, and so on, read the Autodocs.
Quote:
Its like saying on code which done for morphos and have pure a(), its "Bullshit" because didn't use IFace->b(), because IFace already in use (but it didn't, as it done for mos).
It's not what I'm saying, its bullshit because it's a outdated way to do things on AmigaOS4. It might be the only way to do it on MorphOS, but this not MorphOS your using now, your running AmigaOS4, and so you need to make the changes so that memory that should be shared is shared, and memory that should be private is private (or shared if you do not know what its used for).
Quote:
IFace->b(), because IFace already in use (but it didn't, as it done for mos).
I'm useing it to explain where the function comes from, I think it should be easier to understand, It might be that you do not need it.
The primary reason way you should be using IExec->, IDOS-> and so on is to prevent function name confusion between CLIB/NEW lib and native Amiga Libraries.
Its not incorrect to use -D__USE_INLINE__ but you need to be careful not to not call the wrong NEWLIB/CLIB function when you want to call a AmigaOS4 library function.
Working on MorphOS is no guaranty its work the same way on AmigaOS4.
Edited by LiveForIt on 2014/3/1 14:48:14 Edited by LiveForIt on 2014/3/1 14:55:07
Strange why it ok on mos and didn't on os4 (i mean why on exit its NULL on mos).
Perhaps the morphos shell or startup code sets the tc_UserData to NULL on startup and exit.
Unless you can find a point in the code where tc_UserData is set back to NULL then you might check why it isn't executed for AmigaOS
Other wise I'd just recommend you save and restore tc_UserData for the main task on start and exit.
ie
int main() { struct Task *this = FindTask(NULL);
APTR olduserdata = this->tc_UserData;
this->tc_UserData = NULL;
...
rest of main
...
this->tc_UserData = olduserdata;
}
I'm not sure I would worry about changing the list allocation to AllocSysObject() as these are private lists and never passed to the OS.
If the code never frees those lists and relies on mallocs cleanup that's a little bit naughty as the memory never gets released till program quit, you'd have to run Odyssey for quite a while before it counted as a seriouslt memeory leak though!
As long as it's only in OS(MORPHOS) parts and you don't add the same in OS(AMIGAOS) parts, which of course couldn't work, it doesn't matter. Either you are currently building Odyssey with USE_PTHREADS enabled, or this file isn't used at all or you'd get a compiler error from #error Need a way to compare threads on this platform
@Joerg All code i build with -D__MORPHOS__ , and make necessary changes in platform related includes, so all IF(MORPHOS) or so, will be in use for os4 as well (there is plenty of files with checking on MORPHOS, and they all in use and have code done by Fab, so disabling it will broken everything).
I.e. in BASE/wtf/Plaform.h your old defines for aos4 from your old owb are commented, and i redefine morphos ones to os4 ones. That necessary to have all specific fab's amiga code (as there a lot, you can see it even in those files i shown, but there is lots more) to build and make it all works as excepted (with those fixes which was done and which we do now).
In other words, if something in if MORPHOS block, it compiled for os4 now. It was done for first 1.9 port years ago, same for 1.16 and now for 1.23. Removing -D__MORPHOS__ from compiling will broken everything, and i will need to reimplemnt a looot of code, which i do not want to do, and main reasson to port (one of them), its exactly to use all the mos code as it "more or less the same as for os3/os4, just need to fix and adapt some parts". And the method prove to be ok with 1.9 and 1.16 already.
But i will check exactly that part with some warning tomorrow, just in case.