Re: pgAdmin III: Muliple SQL tabs

From: Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Sergey Busel <sbusel(at)gmail(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Subject: Re: pgAdmin III: Muliple SQL tabs
Date: 2016-02-19 05:17:05
Message-ID: CACCA4P2G29nYSEjFzOOnO6uSBG3x0efm8fitKqA4kZpOvHc=dg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

I re-tested and reviewed the attached patch and it is working perfectly.
Thank you Sergey for the patch.

Thanks,
Neel Patel

On Thu, Feb 18, 2016 at 9:08 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi Sergey
>
> That works perfectly for me! I've made a minor change in the attached
> patch - the New button on the button bar now creates a new tab rather than
> a new window. That seems more useful to me (you still have options for both
> on the File menu). Seem OK to you?
>
> Neel - can you please re-test the attached patch and give it a second
> review?
>
> Thanks.
>
> On Tue, Feb 16, 2016 at 7:53 PM, Sergey Busel <sbusel(at)gmail(dot)com> wrote:
>
>> Dave,
>>
>> I still could not recreate the crash in my environment. But I have
>> modified the way tabs are closed. Tabs are no longer being removed/deleted
>> from the code, that task is now left for AuiNotebook when the window is
>> closed. Again, it works for me without crashes. I am testing with up to 15,
>> 20, 25 tabs...
>>
>> Could you please test the attached patch and see if not deleting tabs
>> from the close event makes any difference?
>>
>> Thank you.
>>
>> On Tue, Feb 16, 2016 at 4:20 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Tue, Feb 16, 2016 at 12:42 AM, Sergey Busel <sbusel(at)gmail(dot)com> wrote:
>>>
>>>> After trying it couple times, I was able to reproduce the crash. I
>>>> think it was due to the way tabs were being closed. A dirty tab would
>>>> prompt the user to save the changes and close the tab right away before
>>>> moving on to the next tab. I think it crashed because the user was closing
>>>> the window via a menu item and wx wanted to put the focus back into the
>>>> last control that had it, which was already closed and cleaned up by the
>>>> AuiNotebook. I could not reproduce this issue when closing the window via
>>>> the X button.
>>>>
>>>> Anyway, I changed the way tabs are closed. First, all tabs are checked
>>>> for being dirty and each dirty tab prompts the user to save the changes.
>>>> Then, all tabs are closed, one at a time. I tried the same steps 10 times
>>>> and could not reproduce the crash.
>>>>
>>>> Please let me know if you are still able to reproduce the crash.
>>>>
>>>
>>> Was that the correct patch? I still see what appears to be the same
>>> crash :-(
>>>
>>> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
>>> 0 libsystem_kernel.dylib 0x9ae66332 __kill + 10
>>> 1 libsystem_kernel.dylib 0x9ae65932 kill$UNIX2003 + 32
>>> 2 libsystem_c.dylib 0x955cf75e raise + 26
>>> 3 libwx_base_carbonud-2.8.0.dylib 0x02477a92 wxTrap() + 18
>>> 4 libwx_macud_core-2.8.0.dylib 0x01db2ceb
>>> wxGUIAppTraitsBase::ShowAssertDialog(wxString const&) + 251
>>> 5 libwx_base_carbonud-2.8.0.dylib 0x02477d41
>>> _ZL16ShowAssertDialogPKwiS0_S0_S0_P11wxAppTraits + 545
>>> 6 libwx_base_carbonud-2.8.0.dylib 0x024780bd
>>> wxAppConsole::OnAssertFailure(wchar_t const*, int, wchar_t const*, wchar_t
>>> const*, wchar_t const*) + 99
>>> 7 libwx_base_carbonud-2.8.0.dylib 0x02477f05 wxOnAssert(wchar_t
>>> const*, int, char const*, wchar_t const*, wchar_t const*) + 309
>>> 8 libwx_macud_core-2.8.0.dylib 0x01dd1c4d
>>> wxControlContainer::SetLastFocus(wxWindow*) + 147
>>> 9 libwx_macud_core-2.8.0.dylib 0x01eda8c0
>>> wxPanel::OnChildFocus(wxChildFocusEvent&) + 50
>>> 10 libwx_base_carbonud-2.8.0.dylib 0x02477984
>>> wxAppConsole::HandleEvent(wxEvtHandler*, void (wxEvtHandler::*)(wxEvent&),
>>> wxEvent&) const + 102
>>> 11 libwx_base_carbonud-2.8.0.dylib 0x02560023
>>> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
>>> wxEvtHandler*, wxEvent&) + 391
>>> 12 libwx_base_carbonud-2.8.0.dylib 0x025624ef
>>> wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 221
>>> 13 libwx_base_carbonud-2.8.0.dylib 0x02561716
>>> wxEvtHandler::ProcessEvent(wxEvent&) + 388
>>> 14 libwx_base_carbonud-2.8.0.dylib 0x02560bc8
>>> wxEvtHandler::ProcessPendingEvents() + 302
>>> 15 libwx_base_carbonud-2.8.0.dylib 0x024786d8
>>> wxAppConsole::ProcessPendingEvents() + 232
>>> 16 libwx_base_carbonud-2.8.0.dylib 0x02555ea3
>>> wxMacProcessNotifierAndPendingEvents + 35
>>> 17 libwx_macud_core-2.8.0.dylib 0x01c9a2c1
>>> wxApp::MacHandleOneEvent(void*) + 97
>>> 18 libwx_macud_core-2.8.0.dylib 0x01c9a3ce wxApp::MacDoOneEvent() +
>>> 246
>>> 19 libwx_macud_core-2.8.0.dylib 0x01ccb7fd wxEventLoop::Dispatch() +
>>> 57
>>> 20 libwx_macud_core-2.8.0.dylib 0x01dff6c1 wxEventLoopManual::Run()
>>> + 421
>>> 21 libwx_macud_core-2.8.0.dylib 0x01db35b4 wxAppBase::MainLoop() + 98
>>> 22 libwx_macud_core-2.8.0.dylib 0x01db2a6a wxAppBase::OnRun() + 52
>>> 23 libwx_base_carbonud-2.8.0.dylib 0x024d8782 wxEntry(int&, wchar_t**)
>>> + 258
>>> 24 libwx_base_carbonud-2.8.0.dylib 0x024d896d wxEntry(int&, char**) +
>>> 77
>>> 25 org.postgresql.pgadmin3 0x00058264 main + 36
>>> (pgAdmin3.cpp:126)
>>> 26 org.postgresql.pgadmin3 0x00036615 start + 53
>>>
>>>
>>>
>>>>
>>>> Thanks.
>>>>
>>>> On Sun, Feb 14, 2016 at 11:55 PM, Neel Patel <
>>>> neel(dot)patel(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi ,
>>>>>
>>>>> I just applied the patch in Linux environment and found the crash.
>>>>> Below are the steps to reproduce.
>>>>> We may need to perform below steps multiple time to reproduce the
>>>>> crash.
>>>>>
>>>>> - Open the "Query Window".
>>>>> - Go to "File" and click on "New SQL tab". Open the 3-4 SQL tabs
>>>>> and execute any query in one SQL tab.
>>>>> - Go to "File" menu and click on "Exit".
>>>>> - After clicking on "Exit" button only Query window should close
>>>>> but pgAdmin3 is getting crashed.
>>>>>
>>>>> Below are the traces for reference.
>>>>>
>>>>> #0 0x00007ffff6c4fbf3 in wxControlContainer::SetLastFocus(wxWindow*)
>>>>> () from /usr/local/lib/libwx_gtk2u_core-2.8.so.0
>>>>> #1 0x00007ffff6ccb684 in wxPanel::OnChildFocus(wxChildFocusEvent&) ()
>>>>> from /usr/local/lib/libwx_gtk2u_core-2.8.so.0
>>>>> #2 0x00007ffff633d9d6 in
>>>>> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
>>>>> wxEvtHandler*, wxEvent&) ()
>>>>> from /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #3 0x00007ffff633da7b in wxEventHashTable::HandleEvent(wxEvent&,
>>>>> wxEvtHandler*) () from /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #4 0x00007ffff633dde7 in wxEvtHandler::ProcessEvent(wxEvent&) () from
>>>>> /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #5 0x00007ffff633d938 in wxEvtHandler::ProcessPendingEvents() () from
>>>>> /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #6 0x00007ffff62bb281 in wxAppConsole::ProcessPendingEvents() () from
>>>>> /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #7 0x00007ffff6c4504e in wxAppBase::ProcessIdle() () from
>>>>> /usr/local/lib/libwx_gtk2u_core-2.8.so.0
>>>>> #8 0x00007ffff6bbfe31 in wxapp_idle_callback () from
>>>>> /usr/local/lib/libwx_gtk2u_core-2.8.so.0
>>>>> #9 0x00007ffff366ace5 in g_main_context_dispatch () from
>>>>> /lib/x86_64-linux-gnu/libglib-2.0.so.0
>>>>> #10 0x00007ffff366b048 in ?? () from
>>>>> /lib/x86_64-linux-gnu/libglib-2.0.so.0
>>>>> #11 0x00007ffff366b30a in g_main_loop_run () from
>>>>> /lib/x86_64-linux-gnu/libglib-2.0.so.0
>>>>> #12 0x00007ffff4106eb2 in gtk_dialog_run () from
>>>>> /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
>>>>> #13 0x00007ffff6c31aea in wxMessageDialog::ShowModal() () from
>>>>> /usr/local/lib/libwx_gtk2u_core-2.8.so.0
>>>>> #14 0x00000000006b79bf in frmQuery::CheckChanged(bool) ()
>>>>> #15 0x00000000006a967e in frmQuery::SqlBookClose(bool) ()
>>>>> #16 0x00000000006b7d1e in frmQuery::OnClose(wxCloseEvent&) ()
>>>>> #17 0x00007ffff633d9d6 in
>>>>> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
>>>>> wxEvtHandler*, wxEvent&) ()
>>>>> from /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #18 0x00007ffff633da7b in wxEventHashTable::HandleEvent(wxEvent&,
>>>>> wxEvtHandler*) () from /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #19 0x00007ffff633dde7 in wxEvtHandler::ProcessEvent(wxEvent&) () from
>>>>> /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #20 0x00007ffff633dd70 in wxEvtHandler::ProcessEvent(wxEvent&) () from
>>>>> /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #21 0x00007ffff6cac39c in wxWindowBase::Close(bool) () from
>>>>> /usr/local/lib/libwx_gtk2u_core-2.8.so.0
>>>>> #22 0x00007ffff633d9d6 in
>>>>> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
>>>>> wxEvtHandler*, wxEvent&) ()
>>>>> from /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #23 0x00007ffff633da7b in wxEventHashTable::HandleEvent(wxEvent&,
>>>>> wxEvtHandler*) () from /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #24 0x00007ffff633dde7 in wxEvtHandler::ProcessEvent(wxEvent&) () from
>>>>> /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #25 0x00007ffff633dd70 in wxEvtHandler::ProcessEvent(wxEvent&) () from
>>>>> /usr/local/lib/libwx_baseu-2.8.so.0
>>>>> #26 0x00007ffff6c2f7b5 in gtk_menu_clicked_callback () from
>>>>> /usr/local/lib/libwx_gtk2u_core-2.8.so.0
>>>>> #27 0x00007ffff3b5b3b8 in g_closure_invoke () from
>>>>> /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
>>>>>
>>>>> We will check for Mac OS X environment and keep you updated.
>>>>>
>>>>> Thanks,
>>>>> Neel Patel
>>>>>
>>>>>
>>>>> On Fri, Feb 12, 2016 at 8:51 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Feb 11, 2016 at 2:22 PM, Sergey Busel <sbusel(at)gmail(dot)com>
>>>>>> wrote:
>>>>>>
>>>>>>> Added the query name to history entries. Please note that history
>>>>>>> entries are not modified if user saves [Query 1] to file [foo.sql]. I don't
>>>>>>> think that would be practical.
>>>>>>>
>>>>>>
>>>>>> Thanks. So I was just about to commit this, when it crashed on me. It
>>>>>> seems to happen only if there are at least 2 non-dirty tabs open (easy to
>>>>>> reproduce - open the query tool, add a second tab, then close the window).
>>>>>> I've fiddled around for a couple of hours, but not been able to get to the
>>>>>> bottom of it despite trying a few tricks that sometimes help avoid weird
>>>>>> issues with wxWidgets. There's a stack trace below.
>>>>>>
>>>>>> Ashesh, Neel, Sanket, Akshay - can any of you take a quick look on an
>>>>>> OSX system? I've attached a slightly cleaned up patch, removing some
>>>>>> commented code and making it a git-diff against master.
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
>>>>>> 0 org.postgresql.pgadmin3 0x0036ce4f
>>>>>> wxWindowBase::GetParent() const + 15 (window.h:590)
>>>>>> 1 libwx_macud_core-2.8.0.dylib 0x01ea6c0b
>>>>>> wxControlContainer::SetLastFocus(wxWindow*) + 81
>>>>>> 2 libwx_macud_core-2.8.0.dylib 0x01faf8c0
>>>>>> wxPanel::OnChildFocus(wxChildFocusEvent&) + 50
>>>>>> 3 libwx_base_carbonud-2.8.0.dylib 0x0254b984
>>>>>> wxAppConsole::HandleEvent(wxEvtHandler*, void (wxEvtHandler::*)(wxEvent&),
>>>>>> wxEvent&) const + 102
>>>>>> 4 libwx_base_carbonud-2.8.0.dylib 0x02634023
>>>>>> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
>>>>>> wxEvtHandler*, wxEvent&) + 391
>>>>>> 5 libwx_base_carbonud-2.8.0.dylib 0x026364ef
>>>>>> wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 221
>>>>>> 6 libwx_base_carbonud-2.8.0.dylib 0x02635716
>>>>>> wxEvtHandler::ProcessEvent(wxEvent&) + 388
>>>>>> 7 libwx_base_carbonud-2.8.0.dylib 0x02634bc8
>>>>>> wxEvtHandler::ProcessPendingEvents() + 302
>>>>>> 8 libwx_base_carbonud-2.8.0.dylib 0x0254c6d8
>>>>>> wxAppConsole::ProcessPendingEvents() + 232
>>>>>> 9 libwx_base_carbonud-2.8.0.dylib 0x02629ea3
>>>>>> wxMacProcessNotifierAndPendingEvents + 35
>>>>>> 10 libwx_macud_core-2.8.0.dylib 0x01d6f2c1
>>>>>> wxApp::MacHandleOneEvent(void*) + 97
>>>>>> 11 libwx_macud_core-2.8.0.dylib 0x01d6f3ce wxApp::MacDoOneEvent()
>>>>>> + 246
>>>>>> 12 libwx_macud_core-2.8.0.dylib 0x01da07fd
>>>>>> wxEventLoop::Dispatch() + 57
>>>>>> 13 libwx_macud_core-2.8.0.dylib 0x01ed46c1
>>>>>> wxEventLoopManual::Run() + 421
>>>>>> 14 libwx_macud_core-2.8.0.dylib 0x01e885b4 wxAppBase::MainLoop()
>>>>>> + 98
>>>>>> 15 libwx_macud_core-2.8.0.dylib 0x01e87a6a wxAppBase::OnRun() + 52
>>>>>> 16 libwx_base_carbonud-2.8.0.dylib 0x025ac782 wxEntry(int&,
>>>>>> wchar_t**) + 258
>>>>>> 17 libwx_base_carbonud-2.8.0.dylib 0x025ac96d wxEntry(int&, char**)
>>>>>> + 77
>>>>>> 18 org.postgresql.pgadmin3 0x00128354 main + 36
>>>>>> (pgAdmin3.cpp:126)
>>>>>> 19 org.postgresql.pgadmin3 0x00106705 start + 53
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Feb 10, 2016 at 8:28 AM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Wed, Feb 10, 2016 at 12:14 AM, Sergey Busel <sbusel(at)gmail(dot)com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Dave,
>>>>>>>>>
>>>>>>>>> Here is a patch that does display the query tab name in the title
>>>>>>>>> bar of the output pane. It seems to work with loading and saving the
>>>>>>>>> perspective, too. The trick is to restore the original text on the output
>>>>>>>>> pane before loading and saving the perspective, and put the query name back
>>>>>>>>> in there right after.
>>>>>>>>>
>>>>>>>>> Attached is the full patch, not the diff from the previous patch I
>>>>>>>>> sent. Let me know if you have other suggestions.
>>>>>>>>>
>>>>>>>>
>>>>>>>> That seems to solve the problem nicely :-) - Thanks!
>>>>>>>>
>>>>>>>> It does lead to one other issue that I can see - the History tab on
>>>>>>>> the results pane now shows the history for all query tabs, which seems a
>>>>>>>> little odd given that the other 3 tabs on the results panel are query tab
>>>>>>>> specific. Unless there are better ideas, I think the best way to fix this
>>>>>>>> is to include the current tab name in the history, e.g.
>>>>>>>>
>>>>>>>> -- Executing query [Query 1]:
>>>>>>>> SELECT version();
>>>>>>>> Total query runtime: 16 msec
>>>>>>>> 1 row retrieved.
>>>>>>>>
>>>>>>>> -- Executing query [Query 2]:
>>>>>>>> SELECT * FROM pg_class
>>>>>>>> Total query runtime: 53 msec
>>>>>>>> 311 rows retrieved.
>>>>>>>>
>>>>>>>> -- Executing query [foo.sql]:
>>>>>>>> SELECT * from pg_attribute
>>>>>>>> Total query runtime: 245 msec
>>>>>>>> 2399 rows retrieved.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>>
>>>>>>>>> Regards.
>>>>>>>>>
>>>>>>>>> On Fri, Feb 5, 2016 at 7:46 AM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> On Wed, Feb 3, 2016 at 3:07 AM, Sergey Busel <sbusel(at)gmail(dot)com>
>>>>>>>>>> wrote:
>>>>>>>>>> > - Removed unused/commented code.
>>>>>>>>>> > - Tab names now reflect the file name, if such is associated
>>>>>>>>>> with a tab.
>>>>>>>>>> > - Removed the "Close Tab" menu item. Added X button to the
>>>>>>>>>> active tab.
>>>>>>>>>> > - To tell the user which tab results are related to and to
>>>>>>>>>> avoid messing
>>>>>>>>>> > with perspective, the name of the related tab is now displayed
>>>>>>>>>> as "Data
>>>>>>>>>> > Output [Query 1]" in the output pane. If the tab is named after
>>>>>>>>>> a file, file
>>>>>>>>>> > name will display in the square brackets. If the file name is
>>>>>>>>>> longer then 15
>>>>>>>>>> > chars, it will be truncated to 15 chars.
>>>>>>>>>> >
>>>>>>>>>> > Let me know any other suggestions you may have.
>>>>>>>>>>
>>>>>>>>>> Thanks - this seems to be working nicely for me. It would be good
>>>>>>>>>> if
>>>>>>>>>> the query name could be displayed in the title bar of the output
>>>>>>>>>> pane
>>>>>>>>>> rather than on the results tab (because it really does apply to
>>>>>>>>>> all
>>>>>>>>>> tabs), but I don't see any sensible way to do that given the
>>>>>>>>>> weird way
>>>>>>>>>> that wxAUI stores it's perspectives.
>>>>>>>>>>
>>>>>>>>>> Ashesh, any ideas?
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Dave Page
>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>
>>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Dave Page
>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>> Twitter: @pgsnake
>>>>>>>>
>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2016-02-19 09:28:47 pgAdmin III commit: Allow multiple SQL editor tabs to be used in the Qu
Previous Message Andrey Zhidenkov 2016-02-18 17:27:22 Building a development version of pgadmin