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.
I agree with Breed -- no understanding of what you all are talking about, but nonetheless I find it fascinating. Also, appreciate the efforts very much. Keep it up!
You are doing a big, great job keep time no one here are asking you have to hurry :) ... if some one do can make the port him self, and i think you are the only one who take this strong big huge job
After running it in os3/winuae/kcon shell and it exit from back to shell: tc_UserData are null.
After running it in os4 shell and it exit from back to shell: tc_UserData are value (not null).
After running it in morphos shell and it exit from back to shell: tc_UserData are null.
What mean that tc_UserData auto-cleans in their shell when exit from program running in shell happens, as well as it was like this on os3's kcon too (and i assume on default one too?). Now question is: as it shell, and it the same for os3 as for mos too, then should our os4 shell be fixed to do the same? I mean is there any specification or something which told what for example old os3.x shell do with it, should it worry about at all, or its all up to developer who use tc_UserData. As its clear now, that only os4 shell do not clean tc_UserData on exit, while os3 and mos ones are.
Anyway, its clear now that because of that, Odyssey's code make assumption that on every run tc_UserData are NULL, and we need save and restore tc_UserData on run/exit for os4 (Done), but maybe our shell need fixing too.
Edited by kas1e on 2014/3/3 6:07:25 Edited by kas1e on 2014/3/3 6:08:46
@joerg I also do better look on that MachineStackMarker.cpp, and all that morphos code in no use there, but not because of MORPHOS (as we should be in), but because its all in the ENABLE(JSC_MULTIPLE_THREADS), which is not enabled for my build, and i do not know if it enabled for mos build, but as code is here, seems should be..
I can try to enable pthreads and get rid of mos code in those JSC_MULTITHREADED parts. If all that stuff will make in end faster usage for us. But imho easy and better way its just ENABLE_JSC_MULTIPLE_THREADS=1 and adapt those parts in morphos blocks, as whole code for sure do not want to be mixed.
Btw, your old owb done with -DWTF_USE_PTHREADS=1 and -DENABLE_JSC_MULTIPLE_THREADS=1 or without ?
Edited by kas1e on 2014/3/3 8:55:08 Edited by kas1e on 2014/3/3 8:57:59 Edited by kas1e on 2014/3/3 9:02:42 Edited by kas1e on 2014/3/3 9:09:57
Anyway, its clear now that because of that, Odyssey's code make assumption that on every run tc_UserData are NULL, and we need save and restore tc_UserData on run/exit for os4 (Done), but maybe our shell need fixing too.
Interesting, although it may not be the shell, it could be the c library startup code.
PS (KCON is *not* a shell its a console, the shell is part of DOS and is what actually runs commands etc, the console is where it displays it's output etc)
Btw, your old owb done with -DWTF_USE_PTHREADS=1 and -DENABLE_JSC_MULTIPLE_THREADS=1 or without ?
Which of the old OWB versions, 1.21 or 2.20? The current version of OWB is build with WTF_USE_PTHREADS=1, HAVE_PTHREAD_RWLOCK=0, ENABLE_JSC_MULTIPLE_THREADS=0, ...
struct PPCRegFrame
{
u_int32_t StackGap[4]; /* StackFrame Gap..so a function working
* with the PPCRegFrame as the GPR1 doesn`t * overwrite any contents with a LR store at 4(1)
*/
u_int32_t Version; /* Version of the structure */
u_int32_t Type; /* Type of the regframe */
u_int32_t Flags; /* The filled up registers */
u_int32_t State; /* State of the Thread(only used for Get) */
I.e for first part we need to make aos4 typedef for PlatformThreadRegisters , for second part way to get thread registers and in end need a way to get the stack pointer for another thread.
Sure it can be all different , but whatever. Just even some prototype idea will be ok to start with, and then we can adapt it to working state.
Just when i build morphos version of 1.23, it was disabled by default and i have -DENABLE_JSC_MULTIPLE_THREADS=0 in morphos build as well. But as code is here, i curious if it works on morphos already and enabled ?
News feed
OS4 Feedback forum
OS4Depot Feedback forum
AmiCygnix forum
ABC shell forum
AmiKit forum
Cinnamon Writer forum
CodeBench forum
Digital Universe forum
Dopus 5 forum
E-UAE forum
Gnash forum
Ibrowse forum
JAmiga forum
Odyssey forum
OWB forum
Qt forum
SmartFileSystem forum
Timberwolf forum
TouchDevice forum
TuneNet forum
Unsatisfactory Software forum
Donate
