Hmm, I can't reproduce it now either. Oh well, thanks for checking. Patch applied!
On Thu, Sep 5, 2013 at 6:27 PM, Dinesh Kumar <[email protected]>wrote: > Hi Dave, > > Sorry for the big delay on this issue. > > I have tried to re-produce the above case in mac but unfortunately i am > not able to re-produce it. I have followed the below steps to reproduce the > case. > > 1) Got the new pgAdmin's master branch. > > 2) Applied the given patch. > > 3) Created a new event trigger function under the schema. > > 4) Created new event trigger. > > 5) Modified the created event trigger function's comment from the schema > node. > > 6) Clicked on "OK". > > The above steps are working fine. > > Kindly let me know, if i miss anything here. > > Thanks in advance. > > Dinesh > > -- > *Dinesh Kumar* > Software Engineer > > Ph: +918087463317 > Skype ID: dinesh.kumar432 > www.enterprisedb.co > <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/> > * > Follow us on Twitter* > @EnterpriseDB > > Visit EnterpriseDB for tutorials, webinars, > whitepapers<http://www.enterprisedb.com/resources-community> and > more <http://www.enterprisedb.com/resources-community> > > > On Wed, Jul 31, 2013 at 6:27 PM, Dinesh Kumar < > [email protected]> wrote: > >> Hi Dave, >> >> Thanks for your inputs. >> >> On Wed, Jul 31, 2013 at 6:00 PM, Dave Page <[email protected]> wrote: >> >>> Hi >>> >>> On Tue, Jul 30, 2013 at 5:16 PM, Dinesh Kumar >>> <[email protected]> wrote: >>> > Hi Dave, >>> > >>> > On Mon, Jul 29, 2013 at 9:22 PM, Dave Page <[email protected]> wrote: >>> >> >>> >> Hi >>> >> >>> >> >>> >> On Fri, Jul 26, 2013 at 5:58 PM, Dinesh Kumar >>> >> <[email protected]> wrote: >>> >>> >>> >>> Hi Dave, >>> >>> >>> >>> Thanks for committing the patch. >>> >>> >>> >>> I have found one memory leak issue in the previous implementation and >>> >>> would like to submit this new patch on top of the >>> Pg_Event_Trigger_V5.patch. >>> >>> Please find the below attached patch and valgrind output and let me >>> know >>> >>> your inputs and suggestions. >>> >> >>> >> I have applied this patch and tested in windows and linux with the above >> steps. But, it's not get crashing in windows/linux. >> >> Apologizes, i haven't tested this in Mac. Let me setup pgAdmin build in >> mac, and will try to fix this issue as well. >> >> Thank you once again. >> >> >> >>> >> OK. Some comments: >>> >> >>> >> + if(newData->GetMetaType() == PGM_SCHEMA && >>> !newData->IsCollection()) >>> >> + { >>> >> + doNeedEvtTrgRefresh = true; >>> >> + } >>> >> + >>> >> + // Event trigger's backend functions are at schema level. >>> >> + // Hence, we can consider the Event Triggers are partially belongs >>> to at >>> >> schema level. >>> >> + // So, if any schema got refreshed, we are refreshing the event >>> trigger >>> >> collection as like schema's object. >>> >> + // It's a special case, which effects the schema operations on the >>> event >>> >> triggers as well. >>> >> + // >>> >> + if (doNeedEvtTrgRefresh) >>> >> + { >>> >> + doNeedEvtTrgRefresh = false; >>> >> + >>> >> >>> Refresh(browser->GetObject(browser->GetNextSibling(browser->GetItemParent(browser->GetSelection())))); >>> >> + } >>> >> >>> >> * Why both with the >>> >> doNeedEvtTrgRefresh flag here? As it's not used anywhere else, why not >>> >> just put the Refresh() call into the first conditional? >>> >> >>> > Yes, True. There is no need of Flag doNeedEvtTrgRefresh. >>> > >>> > >>> >> >>> >> * I assume the Refresh call is there to find the "Event Triggers" >>> node and >>> >> refresh it? If so, there's no guarantee that the next sibling will >>> actually >>> >> be the event triggers node - in the future, we may add a new node >>> type that >>> >> sits in that position, or the user may have enabled or disabled some >>> node >>> >> types, including Event Triggers. >>> >> >>> > Ah. Thanks Dave for your suggestions. I have followed another approach >>> to >>> > fix this issue. Kindly check this latest patch and share you inputs and >>> > suggestions. This patch also fixes the memory leak and schema refresh >>> > activities. >>> > >>> >> * The same comment as above applies to browser->GetPrevSibling(). >>> >> >>> > Thanks Dave. >>> >>> I tweaked the patch to clarify the comments and some variable names >>> (see attached), then tested it by changing the comment on an event >>> trigger function from under the schema. I got the following crash >>> after clicking OK: >>> >>> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread >>> 0 libwx_base_carbonu-2.8.0.dylib 0x0158cc8a wxListBase::Clear() + 78 >>> 1 libwx_macu_core-2.8.0.dylib 0x011ae948 >>> wxListLineData::~wxListLineData() + 72 >>> 2 libwx_macu_core-2.8.0.dylib 0x011a7105 >>> wxListMainWindow::DoDeleteAllItems() + 527 >>> 3 libwx_macu_core-2.8.0.dylib 0x011ac54c >>> wxListMainWindow::DeleteEverything() + 120 >>> 4 libwx_macu_core-2.8.0.dylib 0x011ac57b >>> wxGenericListCtrl::ClearAll() + 23 >>> 5 libwx_macu_core-2.8.0.dylib 0x0115bead wxListCtrl::ClearAll() + 27 >>> 6 pgAdmin3-Debug 0x002ba07c frmMain::ResetLists() + 32 >>> 7 pgAdmin3-Debug 0x002ba643 >>> frmMain::execSelChange(wxTreeItemId, bool) + 35 >>> 8 pgAdmin3-Debug 0x002b7723 >>> frmMain::OnTreeSelChanged(wxTreeEvent&) + 61 >>> 9 libwx_base_carbonu-2.8.0.dylib 0x0154b130 >>> wxAppConsole::HandleEvent(wxEvtHandler*, void >>> (wxEvtHandler::*)(wxEvent&), wxEvent&) const + 42 >>> 10 libwx_base_carbonu-2.8.0.dylib 0x015cc25b >>> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&, >>> wxEvtHandler*, wxEvent&) + 125 >>> 11 libwx_base_carbonu-2.8.0.dylib 0x015cd371 >>> wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 221 >>> 12 libwx_base_carbonu-2.8.0.dylib 0x015cca6c >>> wxEvtHandler::ProcessEvent(wxEvent&) + 194 >>> 13 libwx_base_carbonu-2.8.0.dylib 0x015cca83 >>> wxEvtHandler::ProcessEvent(wxEvent&) + 217 >>> 14 libwx_macu_core-2.8.0.dylib 0x0123ceca >>> wxWindowBase::TryParent(wxEvent&) + 66 >>> 15 libwx_base_carbonu-2.8.0.dylib 0x015cca97 >>> wxEvtHandler::ProcessEvent(wxEvent&) + 237 >>> 16 libwx_base_carbonu-2.8.0.dylib 0x015cca83 >>> wxEvtHandler::ProcessEvent(wxEvent&) + 217 >>> 17 libwx_macu_core-2.8.0.dylib 0x0126456c >>> wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) + 40 >>> 18 libwx_macu_core-2.8.0.dylib 0x012723b7 >>> wxGenericTreeCtrl::DoSelectItem(wxTreeItemId const&, bool, bool) + 743 >>> 19 libwx_macu_core-2.8.0.dylib 0x0126e4ec >>> wxGenericTreeCtrl::OnMouse(wxMouseEvent&) + 2866 >>> 20 libwx_base_carbonu-2.8.0.dylib 0x0154b130 >>> wxAppConsole::HandleEvent(wxEvtHandler*, void >>> (wxEvtHandler::*)(wxEvent&), wxEvent&) const + 42 >>> 21 libwx_base_carbonu-2.8.0.dylib 0x015cc25b >>> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&, >>> wxEvtHandler*, wxEvent&) + 125 >>> 22 libwx_base_carbonu-2.8.0.dylib 0x015cd371 >>> wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 221 >>> 23 libwx_base_carbonu-2.8.0.dylib 0x015cca6c >>> wxEvtHandler::ProcessEvent(wxEvent&) + 194 >>> 24 libwx_base_carbonu-2.8.0.dylib 0x015cca83 >>> wxEvtHandler::ProcessEvent(wxEvent&) + 217 >>> 25 libwx_macu_core-2.8.0.dylib 0x0126456c >>> wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) + 40 >>> 26 libwx_macu_core-2.8.0.dylib 0x0118ef16 >>> wxMacTopLevelMouseEventHandler(OpaqueEventHandlerCallRef*, >>> OpaqueEventRef*, void*) + 1286 >>> 27 libwx_macu_core-2.8.0.dylib 0x0118c0e8 >>> wxMacTopLevelEventHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*, >>> void*) + 4344 >>> 28 com.apple.HIToolbox 0x910f39bb >>> _InvokeEventHandlerUPP(OpaqueEventHandlerCallRef*, OpaqueEventRef*, >>> void*, long (*)(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*)) + >>> 36 >>> 29 com.apple.HIToolbox 0x90f7b394 >>> DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, >>> HandlerCallRec*) + 1343 >>> 30 com.apple.HIToolbox 0x90f7a780 >>> SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, >>> HandlerCallRec*) + 430 >>> 31 com.apple.HIToolbox 0x90f8e655 SendEventToEventTarget + 88 >>> 32 com.apple.HIToolbox 0x90fae5c7 >>> ToolboxEventDispatcherHandler(OpaqueEventHandlerCallRef*, >>> OpaqueEventRef*, void*) + 2141 >>> 33 com.apple.HIToolbox 0x90f7b83f >>> DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, >>> HandlerCallRec*) + 2538 >>> 34 com.apple.HIToolbox 0x90f7a780 >>> SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, >>> HandlerCallRec*) + 430 >>> 35 com.apple.HIToolbox 0x90f8e655 SendEventToEventTarget + 88 >>> 36 libwx_macu_core-2.8.0.dylib 0x0112e055 >>> wxApp::MacHandleOneEvent(void*) + 41 >>> 37 libwx_macu_core-2.8.0.dylib 0x0112e148 wxApp::MacDoOneEvent() + 144 >>> 38 libwx_macu_core-2.8.0.dylib 0x0114736e wxEventLoop::Dispatch() + 32 >>> 39 libwx_macu_core-2.8.0.dylib 0x011e7e25 wxEventLoopManual::Run() + >>> 131 >>> 40 libwx_macu_core-2.8.0.dylib 0x011c14cb wxAppBase::MainLoop() + 67 >>> 41 libwx_base_carbonu-2.8.0.dylib 0x01580dae wxEntry(int&, wchar_t**) + >>> 110 >>> 42 libwx_base_carbonu-2.8.0.dylib 0x01580e62 wxEntry(int&, char**) + 50 >>> 43 pgAdmin3-Debug 0x000a768e main + 30 >>> 44 libdyld.dylib 0x95f55725 start + 1 >>> >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >> >> >> >> Dinesh >> >> -- >> *Dinesh Kumar* >> Software Engineer >> >> Ph: +918087463317 >> Skype ID: dinesh.kumar432 >> www.enterprisedb.co >> <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/> >> * >> Follow us on Twitter* >> @EnterpriseDB >> >> Visit EnterpriseDB for tutorials, webinars, >> whitepapers<http://www.enterprisedb.com/resources-community> and >> more <http://www.enterprisedb.com/resources-community> >> >> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
