Re: pgAdmin III: Muliple SQL tabs

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