Re: [Harbour] Re: Pritpal and Vikto, please test this generator
Pritpal, I now think that in prg classes we should add destructor and destroy all the children at harbour level from there, so doing that ourself and not leaving the job to Qt The other test I want to do is to use object.deleteLater() but it is not a solution, is an hack ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
[Harbour] Re: Pritpal and Vikto, please test this generator
Przemysław Czerpak wrote: You may need explicit destructors in .prg code only if you plan to keep some structures which are not released automatically in harbour object. If you correctly implement releasing all QT objects by automatic GC pointer item destructors then you will not need any PRG level destructors to replicate the same job. Instead of all the protections in hbQT's GC pointer items destruction we are still unable to grasp a logic why for some variables behavior is indifferent. I mean we are never sure which another child is destroyed by Qt with its parent and prevent the latter not to be sent to GC engine. BTW the only one thing which should be introduced yet is an option to mark GC pointer items as attached to other QT structures so they should not be released automatically with the HVM item because they will be released with parent object. Few days before I had resuested for this feature, though I admit, I could not explain that in as simple language as above of your paragraph. If possible, please do it. - enjoy hbIDEing... Pritpal Bedi http://hbide.vouch.info/ -- View this message in context: http://n2.nabble.com/Pritpal-and-Vikto-please-test-this-generator-tp4900883p4907702.html Sent from the harbour-devel mailing list archive at Nabble.com. ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
[Harbour] Re: Pritpal and Vikto, please test this generator
Bisz István wrote: BTW the only one thing which should be introduced yet is an option to mark GC pointer items as attached to other QT structures so they should not be released automatically with the HVM item because they will be released with parent object. Yes, indeed, this is the solution for the so hunted GPF-s in HBQT. In the attachment is the hbqt patch for the hbide crashes in different circumstances. The problem is generated by the following sequences in searchreplace.uic: horizontalSpacer = new QSpacerItem(40, 20, QSizePolicy::Expanding, QSizePolicy::Minimum); horizontalLayout-addItem(horizontalSpacer); and horizontalSpacer_2 = new QSpacerItem(40, 20, QSizePolicy::Expanding, QSizePolicy::Minimum); horizontalLayout_2-addItem(horizontalSpacer_2); The QSpacerItem is not in inherited from QObject and in this way Qt has no information about the deletion of the object executed by hbqt. After deletion of the QSpacerItem by hbqt the subsequent delete of the horizontalLayout or horizontalLayout_2 generates in Qt engine the deletion of the child object, in our case the already deleted QSpacerItem...crash. The attachment contains: a. QLayout.cpp the generated cpp file. b. QLayout.qth with the modified QT_QLAYOUT_ADDITEM: CODE ... HB_FUNC( QT_QLAYOUT_ADDITEM ) { QGC_POINTER * p; QGC_POINTER * q; HB_TRACE( HB_TR_DEBUG, ( Entering function QT_QLAYOUT_ADDITEM() ) ); q = ( QGC_POINTER * ) hb_parptrGC( hbqt_gcFuncs(), 1 ); p = ( QGC_POINTER * ) hb_parptrGC( hbqt_gcFuncs(), 2 ); if( p p-ph q q-ph ) { HB_TRACE( HB_TR_DEBUG, ( QT_QLAYOUT_ADDITEM() Qt oject: %p is attached to: %p, p-ph, q-ph ) ); p-bNew = HB_FALSE; } hbqt_par_QLayout( 1 )-addItem( hbqt_par_QLayoutItem( 2 ) ); } ... /CODE c. THbQtUI.prg with the corrected sequence: ... HB_TRACE( HB_TR_ALWAYS, 101 ) ::oWidget:close() HB_TRACE( HB_TR_ALWAYS, 102 ) ::oWidget := NIL HB_TRACE( HB_TR_ALWAYS, 103 ) hbide_justACall( i ) RETURN NIL ... Welcome back, we really missed you. Though above solution is exactly what effectively handles the situation, and you have well described it, it is not the only part where it may fail. Such scenarios can be many, especially when we are handelling .uic files at the prg level. Also it requires a manual update of such methods on .qth levels, which in my opinion is not the right solution. Przemek's second option is to introduce GC items dependant on some other is the right direction to go. - enjoy hbIDEing... Pritpal Bedi http://hbide.vouch.info/ -- View this message in context: http://n2.nabble.com/Pritpal-and-Vikto-please-test-this-generator-tp4900883p4907776.html Sent from the harbour-devel mailing list archive at Nabble.com. ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
[Harbour] Re: Pritpal and Vikto, please test this generator
Bisz István wrote: Yes, indeed, this is the solution for the so hunted GPF-s in HBQT. In the attachment is the hbqt patch for the hbide crashes in different circumstances. The problem is generated by the following sequences in searchreplace.uic: horizontalSpacer = new QSpacerItem(40, 20, QSizePolicy::Expanding, QSizePolicy::Minimum); horizontalLayout-addItem(horizontalSpacer); and horizontalSpacer_2 = new QSpacerItem(40, 20, QSizePolicy::Expanding, QSizePolicy::Minimum); horizontalLayout_2-addItem(horizontalSpacer_2); For now I am including above patch in my tests. And if it resolves crashes with the updated code, I will commit. Later we will be waiting for Przemek's commit to see it the same is done on GC levels. - enjoy hbIDEing... Pritpal Bedi http://hbide.vouch.info/ -- View this message in context: http://n2.nabble.com/Pritpal-and-Vikto-please-test-this-generator-tp4900883p4907848.html Sent from the harbour-devel mailing list archive at Nabble.com. ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
RE: [Harbour] Re: Pritpal and Vikto, please test this generator
Hi Pritpal, For now I am including above patch in my tests. And if it resolves crashes with the updated code, I will commit. Later we will be waiting for Przemek's commit to see it the same is done on GC levels. OK, sounds good. In any case if it fits to our needs is better than nothing temporarily. It's sufficient to solve the scenario just for the linked non QObject-s. In the future we should switch the QPointer-s to QWeakPointer-s, covering in this way probably the non QObject-s too, but this looks to be a long way. Best regards, István ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
[Harbour] Re: Pritpal and Vikto, please test this generator
Bisz István wrote: HB_FUNC( QT_QLAYOUT_ADDITEM ) { QGC_POINTER * p; QGC_POINTER * q; HB_TRACE( HB_TR_DEBUG, ( Entering function QT_QLAYOUT_ADDITEM() ) ); q = ( QGC_POINTER * ) hb_parptrGC( hbqt_gcFuncs(), 1 ); p = ( QGC_POINTER * ) hb_parptrGC( hbqt_gcFuncs(), 2 ); if( p p-ph q q-ph ) { HB_TRACE( HB_TR_DEBUG, ( QT_QLAYOUT_ADDITEM() Qt oject: %p is attached to: %p, p-ph, q-ph ) ); p-bNew = HB_FALSE; } hbqt_par_QLayout( 1 )-addItem( hbqt_par_QLayoutItem( 2 ) ); } ... /CODE c. THbQtUI.prg with the corrected sequence: ... HB_TRACE( HB_TR_ALWAYS, 101 ) ::oWidget:close() HB_TRACE( HB_TR_ALWAYS, 102 ) ::oWidget := NIL HB_TRACE( HB_TR_ALWAYS, 103 ) hbide_justACall( i ) RETURN NIL ... Did you test this code ? I think, yes. Then prg level :addItem() method is missing, how you solved that? Manually writing in TQLayout.prg ? - enjoy hbIDEing... Pritpal Bedi http://hbide.vouch.info/ -- View this message in context: http://n2.nabble.com/Pritpal-and-Vikto-please-test-this-generator-tp4900883p4908219.html Sent from the harbour-devel mailing list archive at Nabble.com. ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
Re: [Harbour] Re: Pritpal and Vikto, please test this generator
Did you test this code ? I think, yes. Then prg level :addItem() method is missing, how you solved that? Manually writing in TQLayout.prg ? I have this function in TQLayout.prg ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
RE: [Harbour] Re: Pritpal and Vikto, please test this generator
Hi Pritpal, Did you test this code ? I think, yes. Yes, you are right, and the crash is gone on my Fedora12. Then prg level :addItem() method is missing, how you solved that? Manually writing in TQLayout.prg ? I replaced just QLayout.cpp. Best regards, István ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
Re: [Harbour] Re: Pritpal and Vikto, please test this generator
Hi to everybody, I tried to solve the problem starting from the end, the delete... I wanted to intercept the delete and from the pointer going back to the harbour QGC_POINTER and nullify it From the end you cover all cases in a generic way.. Your solution is from another point of view, you disable the delete statement when GC destructor is called on children so the only way to REALLY destroy a child is to wait for parent deletion... this may solve some problems but, as said and as I understood your sample, we may have deveral hanging objects... Is it also necessary to implement the same logic for addWidget, but also a inverted logic for removeWidget and removeItem... More, we need to devise a standard way to generate such code from qth/include files from the actual generator I think it is very difficult, we should create an in-memory structure of calls and then generate code as automatically as possible... Anyway, congratulations for the finding... Francesco ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
RE: [Harbour] Re: Pritpal and Vikto, please test this generator
Hi Francesco, More, we need to devise a standard way to generate such code from qth/include files from the actual generator I think it is very difficult, we should create an in-memory structure of calls and then generate code as automatically as possible... Yes, you are right, this patch is just the first step in the crash hunting. The next steps should be oriented to a more general dynamic approach. Best regards, István ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
Re: [Harbour] Re: Pritpal and Vikto, please test this generator
Yes, you are right, this patch is just the first step in the crash hunting. The next steps should be oriented to a more general dynamic approach. This is an important patch because it started a new way for solving GPF... Now there are several protections against GPFs... - guarded pointers for objects deriving from QObject - your way when objects are parented at runtime - second parameter of *_gcAllocate_* set to false Still missing is full check on parameter types but Przemek already gave some ideas... Francesco ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
Re: [Harbour] Re: Pritpal and Vikto, please test this generator
Istvan, in hbide.prg please comment all the :destroy() lines, there are a bunch together... I will try later... Francesco ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
Re: [Harbour] Re: Pritpal and Vikto, please test this generator
Sorry Pritpal, you were correct. In the QLayout.qth file proposed by Istvan the line of addItem is commented and so the harbour class is not generated... Francesco ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
Re: [Harbour] Re: Pritpal and Vikto, please test this generator
A bit more tracing in the debug log diff -r 020effeac192 harbour/contrib/hbqt/generatorF/hbqtgen.prg --- a/harbour/contrib/hbqt/generatorF/hbqtgen.prg Thu Apr 15 09:56:57 2010 +0200 +++ b/harbour/contrib/hbqt/generatorF/hbqtgen.prg Thu Apr 15 22:52:42 2010 +0200 @@ -50,6 +50,8 @@ * */ /*--*/ + +#define TRACE_QT_CALLS #include common.ch #include fileio.ch @@ -1237,7 +1239,9 @@ aadd( txt_, + cWidget + iif( lList, void *, ) + * p; ) aadd( txt_, ) aadd( txt_,p = hbqt_par_ + cWidget + ( 1 ); ) - +#ifdef TRACE_QT_CALLS + aadd( txt_, 'HB_TRACE( ' + s_trMode + ', (' + upper( cWidget ) + '_' + upper( cHBFunc) + ' ) );') +#endif /* Insert parameters by reference */ IF ! empty( aPre ) FOR n := 1 TO len( aPre ) ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
RE: [Harbour] Re: Pritpal and Vikto, please test this generator
Francesco, Since I'm trying to implement your changes in the generator, if you have a better version please send... My post doesn't contain any other info. I am happy if you can find the solution in the generator part. István ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
Re: [Harbour] Re: Pritpal and Vikto, please test this generator
Since I got GPF in another point (removing the :destroy() part) of the code (releasing a QToolBar) I used the trace above to see what function calls were done on that QToolBar... they are addAction toggleViewAction setIcon at least one of them must have the change you proposed Francesco ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
RE: [Harbour] Re: Pritpal and Vikto, please test this generator
Since I got GPF in another point (removing the :destroy() part) of the code (releasing a QToolBar) I used the trace above to see what function calls were done on that QToolBar... they are addAction toggleViewAction setIcon at least one of them must have the change you proposed My experience is that the Qt objects should be released by a controlled pointer zeroing in the prg level, otherwise we are facing to GPF-s (not analyzed harbour internals...). ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
Re: [Harbour] Re: Pritpal and Vikto, please test this generator
Ok, just for trying to understand how it is possible to proceed and to validate the idea I did some tests... I was able to remove a GPFs changing some add* functions. It was a manual work and I strongly believe it can't be done manually ! Just for reference I post here one of this changed functions, for code review: HB_FUNC( QT_QMAINWINDOW_ADDTOOLBAR ) { QGC_POINTER_QMainWindow * q; QGC_POINTER * p; HB_TRACE( HB_TR_DEBUG, (QMAINWINDOW_ADDTOOLBAR ) ); q = ( QGC_POINTER_QMainWindow * ) hb_parptrGC( hbqt_gcFuncs(), 1 ); p = ( QGC_POINTER * ) hb_parptrGC( hbqt_gcFuncs(), 3 ); if ( p p-ph q q-ph ) { HB_TRACE( HB_TR_DEBUG, ( QT_QMAINWINDOW_ADDTOOLBAR() Qt oject: %p is attached to: %p, (void *) p-ph, (void *) q-ph) ); p-bNew = HB_FALSE; if ( q q-ph ) ( q-ph )-addToolBar( ( Qt::ToolBarArea ) hb_parni( 2 ), ( ( QToolBar *) p-ph )); else HB_TRACE( HB_TR_DEBUG, ( F=QT_QMAINWINDOW_ADDACTION FP=( q-ph )-addAction( xx ); q-ph is NULL )); } } Francesco ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
[Harbour] Re: Pritpal and Vikto, please test this generator
francesco perillo wrote: Ok, just for trying to understand how it is possible to proceed and to validate the idea I did some tests... I was able to remove a GPFs changing some add* functions. It was a manual work and I strongly believe it can't be done manually ! Just for reference I post here one of this changed functions, for code review: HB_FUNC( QT_QMAINWINDOW_ADDTOOLBAR ) { QGC_POINTER_QMainWindow * q; QGC_POINTER * p; HB_TRACE( HB_TR_DEBUG, (QMAINWINDOW_ADDTOOLBAR ) ); q = ( QGC_POINTER_QMainWindow * ) hb_parptrGC( hbqt_gcFuncs(), 1 ); p = ( QGC_POINTER * ) hb_parptrGC( hbqt_gcFuncs(), 3 ); if ( p p-ph q q-ph ) { HB_TRACE( HB_TR_DEBUG, ( QT_QMAINWINDOW_ADDTOOLBAR() Qt oject: %p is attached to: %p, (void *) p-ph, (void *) q-ph) ); p-bNew = HB_FALSE; if ( q q-ph ) ( q-ph )-addToolBar( ( Qt::ToolBarArea ) hb_parni( 2 ), ( ( QToolBar *) p-ph )); else HB_TRACE( HB_TR_DEBUG, ( F=QT_QMAINWINDOW_ADDACTION FP=( q-ph )-addAction( xx ); q-ph is NULL )); } } If we go this way, i.e., manually changing the code, then probably we can never finish, nor anybody will take interest in. There can be thousands of combinations like this. We should wait until Przemek commits detachable GC pointers. - enjoy hbIDEing... Pritpal Bedi http://hbide.vouch.info/ -- View this message in context: http://n2.nabble.com/Pritpal-and-Vikto-please-test-this-generator-tp4900883p4910638.html Sent from the harbour-devel mailing list archive at Nabble.com. ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
[Harbour] Re: Pritpal and Vikto, please test this generator
francesco perillo wrote: I continued to do some work on the generator and implemented the check on -calls From: HB_FUNC( QT_QAPPLICATION_CURSORFLASHTIME ) { hb_retni( hbqt_par_QApplication( 1 )-cursorFlashTime() ); } To: HB_FUNC( QT_QAPPLICATION_CURSORFLASHTIME ) { QApplication * p ; p = hbqt_par_QApplication( 1 ) ; if( p ) hb_retni( ( p )-cursorFlashTime() ); else HB_TRACE. } Francesco, carry on. I am onto it and have spent few hours on your previous upload. I have noticed some points. Now I will include this patch also and will continue to experiment. I will forward detailed analysis later. I am not replying you because I do not want to influence your line of thoughts. BTW you are heading towards right direction. Try to implement concept like this ( Przemek's code ): pObj = ( QPointer QPageSetupDialog * ) memset( hb_gcAllocate( sizeof( QPointer QPageSetupDialog ), s_gcQPageSetupDialog ), 0, sizeof( QPointer QPageSetupDialog ) ); * pObj = obj; - enjoy hbIDEing... Pritpal Bedi http://hbide.vouch.info/ -- View this message in context: http://n2.nabble.com/Pritpal-and-Vikto-please-test-this-generator-tp4900883p4901678.html Sent from the harbour-devel mailing list archive at Nabble.com. ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
[Harbour] Re: Pritpal and Vikto, please test this generator
francesco perillo wrote: pObj = ( QPointer QPageSetupDialog * ) memset( hb_gcAllocate( sizeof( QPointer QPageSetupDialog ), s_gcQPageSetupDialog ), 0, sizeof( QPointer QPageSetupDialog ) ); It is already included it is visually different as we use object and not *object and we gcAllocate QGC_OBJECT and not directly the object... but the result is the same (should be :-) ) Sounds correct. What really surprised me is the not getting a GPF when ( p) -funz() and p is NULL. Yep, and I am surprised too. BTW this patch has provided more insight in how some objects must behave with singal/slots and events. Check for _new_QTableWidgetItem and then ProcessEvnts() firing. This is something I have to fix. I will resume the work later this evening... I miss one point: hbqt_garbage include extern ... ( void *, iParam) I already changed void * to the correct class but then I should include all the headers... I'm in a hurry I will explain better later.. Keep on, hopefully we will be on target soon. - enjoy hbIDEing... Pritpal Bedi http://hbide.vouch.info/ -- View this message in context: http://n2.nabble.com/Pritpal-and-Vikto-please-test-this-generator-tp4900883p4902218.html Sent from the harbour-devel mailing list archive at Nabble.com. ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour
Re: [Harbour] Re: Pritpal and Vikto, please test this generator
I'm about to implement a -massivedebug switch to the generator sample: HB_FUNC( QT_QPUSHBUTTON_ISFLAT ) { QPushButton * p; HB_TRACE( HB, ( Entering function QT_QPUSHBUTTON_ISFLAT ) ); and with a bit more knowledge on parameters/object (also thanks to Viktor generator2) we may also print the parameter passed and the return value/pointer... I also created my cppstub with some debug info inside... I need to print more infos and then match one new with one delete Ciao Francesco ___ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour