WIP & BUGGY: WebUI: Add additional keyboard handlers (e.g. Insert, Delete, Edit...)#1
Open
ProfYaffle wants to merge 2 commits intomasterfrom
Open
WIP & BUGGY: WebUI: Add additional keyboard handlers (e.g. Insert, Delete, Edit...)#1ProfYaffle wants to merge 2 commits intomasterfrom
ProfYaffle wants to merge 2 commits intomasterfrom
Conversation
ProfYaffle
pushed a commit
that referenced
this pull request
Oct 2, 2018
Although the existing backtrace works correctly on Linux, on
FreeBSD it frequently generates a backtrace with completely
wrong function names. (FreeBSD 11.2, current latest version).
For example, making htsp_build_dvrentry crash with SEGV, it
would either not generate a stacktrace or would generate a
backtrace of:
-pthread_sigmask
-pthread_getspecific
-service_remove_unseen
-htsp_get_subscription_status
-htsp_init
-tcp_server_done
-tvhthread_create.
...instead of the correct backtrace of:
-<signal>
-htsp_build_dvrentry
-htsp_method_async
-htsp_read_loop
-htsp_serve...
So on FreeBSD only, we use libunwind to generate the
backtrace and function names. We explicitly make
libunwind and libexecinfo mutually exclusive since
FreeBSD has both.
Line are logged similar to:
CRASH: htsp_build_dvrentry+5d (ip=11f659d sp=7fffd8bc3930)
Note that it does not have line numbers since the addr2line
does not appear to work on FreeBSD (even with the original
backtrace code).
An example of the problem with the old backtrace code using
the frame from htsp_method_async from within the tvheadend
traphandler after the retrieval of the stack frames:
(gdb) print frames
$38 = {0x806473954, 0x806472eb2, 0x7ffffffff193, 0x11f1638 <htsp_method_async+1640>, 0x11fe400 <htsp_read_loop+880>, 0x11f58e6 <htsp_serve+502>, 0x11b9b11 <tcp_server_start+401>,
0x11af45e <thread_wrapper+302>, 0x80646dc06, 0x0 <repeats 91 times>}
(gdb) print dladdr(0x11f1638, &dli) <--- addr of htsp_method_async from frame 4.
$39 = 1 <--- success
(gdb) print dli
$40 = {dli_fname = 0x7fffffffef97 ".../build.freebsd/tvheadend", dli_fbase = 0x1021000, dli_sname = 0x1044f91 "service_remove_unseen", <--- but wrong name
dli_saddr = 0x11eff80 <service_remove_unseen>} <--- and this is nearest symbol address
(gdb) print htsp_method_async+1640
$41 = (htsmsg_t *(*)(htsp_connection_t *, htsmsg_t *)) 0x11f1638 <htsp_method_async+1640> <---but gdb knows the original address is htsp_method_async
(gdb) print service_remove_unseen
$42 = {void (const char *, int)} 0x11eff80 <service_remove_unseen> <--- and gdb knows sevice_remove_unseen is at the dli_saddr.
By contrast, with libunwind, we get:
(gdb) print buf
$50 = "htsp_method_async", '\000' <repeats 110 times> <--- libunwind detected correct function name
(gdb) where 10 <--- even though our signal has been delivered on its own stack
#0 traphandler_libunwind () at src/trap.c:162
#1 0x000000000120cf06 in traphandler (sig=11, si=0x7fffdbbdb860, UC=0x7fffdbbdb4f0) at src/trap.c:221
#2 0x0000000806673954 in ?? ()
tvheadend#3 0x0000000000000000 in ?? ()
(gdb) print ip
$51 = 18814904
(gdb) disass 18814904 <--- and gdb knows that ip address is for the same method as libunwind detected
Dump of assembler code for function htsp_method_async:
0x00000000011f1150 <+0>: push %rbp
ProfYaffle
pushed a commit
that referenced
this pull request
Apr 5, 2022
…on (#1) * Rework disk space check and cleanup * Update dvr.h * Update dvr_vfsmgr.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
REQUIRES REVIEW/MODIFICATION. DO NOT MERGE.
I'm not happy about this - but, in my defence, I don't know what I'm doing :)
This adds:
(8. Ctrl + a == Selects displayed record(s) - implemented previously)
This seems to (mostly) work, but it feels ugly and is buggy. All debugging statements left in.
Issues I can see:
The grids already use PgUp/PgDn within the displayed page, so we can't (shouldn't) interfere with these. For example, if you have a scrolling page (maybe 100 items) then PgUp/PgDn moves through it, not forwards/backwards a page. Similarly, Home/End move to the top and bottom of the displayed page, not to the top/bottom of the store.
However, these keys do work if the focus is on the TextBox in the Paging Toolbar (where you'd type a new page number). So I've implemented this as a shift-focus keystroke instead of over-riding the keys: Alt+p moves focus to this text box, and then PgUp/PgDn changes page, with Home and End moving to the first and last page respectively.
The way that the tabbed panel works is that a key event passes through all the tabs in turn until it reaches the active one. So, a keystroke in "Failed Recordings" triggers three events: one for Upcoming, one for Finished, and a final one when arriving at Failed. So we need to find a way to limit processing to the current (active) tab.
I've done this by simply checking to see that a grid is rendered in the panel, as it will only be rendered in the active one.
Save/Undo only make sense if there's something to Save or Undo, so I've tested to see if these buttons have been scoped in (enabled) before triggering the appropriate handler. That leaves the grid to decide if anything has changed, rather than checking again.
The same logic should probably hold to the other buttons - Add, Delete. They're normally there, but you can't guaranteed this.
However, there are some serious bugs:
It's a global keymap, so these keystrokes apply everywhere. They should be limited to the DVR screens, or otherwise to tabs that make sense (e.g. this will intercept DEL on any text field, which is utter rubbish - you go to erase, say, the server name in Configuration -> General and it fires a deletion event associated with a non-existent Button).
This highlights that the button check (if !button.disabled) doesn't work. Something in the way the tabs are organised means that somewhere this button is enabled once something has changed, so this test passes. It has to be limited to the active/displayed tab.
A better way might be to simply have the key bindings on the Button objects, but I don't know that's possible. ExtJS 3.4 seems to have overlooked keyboard input for the most part.
There seems to be an issue with "once selected, always selected". So, if you select an item in Upcoming Recordings, press F2 to Edit, dismiss, then tab away to, say, "Failed", then F2 no longer does anything - which is correct, as there's no selected record to Edit.
However, if you go back to Upcoming and press F2 again then the previously-selected record will pop up, even if no rows are selected... if you select a new row, then F2 will pop up a dialog for both of them. This will repeat for every row you select, so something's remembering the selection.
==> this has serious implications for 'Delete' if that would erase any and all previously-selected records on a tab