[-- Attachment #1.1: Type: text/plain, Size: 1165 bytes --] Hi everyone, ZSH_VERSION: 5.8 (Distribution, Devuan, still hasn't update) $ZSH_PATCHLEVEL: debian/5.8-6+deb11u1 default editor nvim version v0.4.4 Also tested with VIM - Vi IMproved 8.2 terminal emulators/multiplexer: tmux, xfce4-terminal, st, and kitty(checked on all) .zshrc autoload -Uz edit-command-line zle -N edit-command-line bindkey -M vicmd v edit-command-line Created an anonymous function(see attachment ps_via_zsh) by entering edit-command-line which seems to work ok after exiting editor and hitting enter. But then(after adding code) started getting out of memory errors(see attachment out_of_memory). When I recall the anonymous function from history and execute, no error. Started commenting out lines and found that the issues stops with the line: CL=${(0A)$(</proc/$P/cmdline)} ; PP=${AA[PPid]} ; Cmd=${AA[Name]} commented out. Splitting line found issue to be with CL=${(0A)$(</proc/$P/cmdline)} Again, recalling from history -- no edit-command-line -- no error. BTW: /proc/<pid>/cmdline uses NULL as the separator. QUESTION: Bug, or am I doing something stupid? Or something with nvim and vim? Thanks for listening, Jim Murphy [-- Attachment #1.2: Type: text/html, Size: 1549 bytes --] [-- Attachment #2: ps_via_zsh --] [-- Type: application/octet-stream, Size: 850 bytes --] () { # Z-Shell version of 'ps -o pid=,ppid=,tty=,comm=,args= $$' emulate -L zsh # FunCTioN: ps_via_zsh [[ -v modules[zsh/datetime] ]] || zmodload zsh/datetime local ST ET ST=$EPOCHREALTIME ps -o pid=,ppid=,tty=,comm=,args= $$ ET=$EPOCHREALTIME print $(( ST - ET )) ST=$EPOCHREALTIME [[ -v modules[zsh/stat] ]] || zmodload zsh/stat local CL Cmd E P PP T local -a A B local -A AA P=$$ T="$(zstat +link /proc/$P/fd/0)" A=(${(M)${${(f)"$(</proc/$P/status)"}:s/:/,}:#(Name|PPid),*}) for E ($A) { B=(${(s.,.)E}) B[2]=${(MS)B[2]##[[:graph:]]*[[:graph:]]} # remove leading/trailing white space AA[$B[1]]="$B[2]" } CL=${(0A)$(</proc/$P/cmdline)} ; PP=${AA[PPid]} ; Cmd=${AA[Name]} print -- ${(l.5.. .)P} ${(l.5.. .)PP} ${(r.12.. .)T} ${(r.15.. .)Cmd} $CL ET=$EPOCHREALTIME print $(( ST - ET )) } [-- Attachment #3: out_of_memory --] [-- Type: application/octet-stream, Size: 1347 bytes --] [ 2065.764363] Out of memory: Killed process 12153 (zsh) total-vm:6812316kB, anon-rss:6802224kB, file-rss:0kB, shmem-rss:0kB, UID:17227 pgtables:13360kB oom_score_adj:0 [ 1681.950581] Out of memory: Killed process 12116 (zsh) total-vm:6818304kB, anon-rss:6802244kB, file-rss:0kB, shmem-rss:0kB, UID:17227 pgtables:13384kB oom_score_adj:0 [23829.634980] Out of memory: Killed process 12375 (zsh) total-vm:5084260kB, anon-rss:5075368kB, file-rss:0kB, shmem-rss:0kB, UID:17227 pgtables:9992kB oom_score_adj:0 [35153.665324] Out of memory: Killed process 12338 (zsh) total-vm:4940764kB, anon-rss:4931592kB, file-rss:0kB, shmem-rss:0kB, UID:17227 pgtables:9712kB oom_score_adj:0 [35777.880940] Out of memory: Killed process 15754 (zsh) total-vm:5090388kB, anon-rss:5081832kB, file-rss:0kB, shmem-rss:0kB, UID:17227 pgtables:10000kB oom_score_adj:0 [36283.820854] Out of memory: Killed process 15709 (zsh) total-vm:5067208kB, anon-rss:5058760kB, file-rss:4kB, shmem-rss:0kB, UID:17227 pgtables:9952kB oom_score_adj:0 [36533.967014] Out of memory: Killed process 30580 (zsh) total-vm:5024676kB, anon-rss:5016104kB, file-rss:4kB, shmem-rss:0kB, UID:17227 pgtables:9868kB oom_score_adj:0 [37206.318183] Out of memory: Killed process 31268 (zsh) total-vm:5024168kB, anon-rss:5012684kB, file-rss:0kB, shmem-rss:0kB, UID:17227 pgtables:9872kB oom_score_adj:0
[-- Attachment #1: Type: text/plain, Size: 2018 bytes --] Hi again, Update: Tried to isolate what causes the 'out of memory' errors and found that if I added quotes the error stopped. CL=${(0A)"$(< /proc/$P/cmdline)"} # OK CL=${(0A)$(< /proc/$P/cmdline)} # out of memory error I had another script and it worked without the quotes. Compared the two and found the other script had the option 'extendedglob' set. After adding the option to the script I attached in the original email the 'out of memory' errors stopped when there are no quotes. Not sure what globbing has to do with this. Hopefully someone can enlighten me. But even so, should zsh ever fail with an 'out of memory' error? Again, it only happens after exiting 'edit-command-line'. Regards, Jim On Mon, Aug 1, 2022 at 10:47 PM Jim <linux.tech.guy@gmail.com> wrote: > Hi everyone, > > ZSH_VERSION: 5.8 (Distribution, Devuan, still hasn't update) > $ZSH_PATCHLEVEL: debian/5.8-6+deb11u1 > default editor nvim version v0.4.4 > Also tested with VIM - Vi IMproved 8.2 > terminal emulators/multiplexer: tmux, xfce4-terminal, st, and > kitty(checked on all) > > .zshrc > autoload -Uz edit-command-line > zle -N edit-command-line > bindkey -M vicmd v edit-command-line > > Created an anonymous function(see attachment ps_via_zsh) by entering > edit-command-line > which seems to work ok after exiting editor and hitting enter. But > then(after adding code) started > getting out of memory errors(see attachment out_of_memory). When I recall > the anonymous > function from history and execute, no error. Started commenting out lines > and found that the issues > stops with the line: > > CL=${(0A)$(</proc/$P/cmdline)} ; PP=${AA[PPid]} ; Cmd=${AA[Name]} > > commented out. Splitting line found issue to be with > CL=${(0A)$(</proc/$P/cmdline)} > Again, recalling from history -- no edit-command-line -- no error. > BTW: /proc/<pid>/cmdline uses NULL as the separator. > > QUESTION: Bug, or am I doing something stupid? Or something with nvim and > vim? > > Thanks for listening, > > Jim Murphy > [-- Attachment #2: Type: text/html, Size: 2921 bytes --]
On Fri, Aug 12, 2022 at 8:41 AM Jim <linux.tech.guy@gmail.com> wrote: > > Tried to isolate what causes the 'out of memory' errors and found that if > I added quotes the error stopped. > CL=${(0A)"$(< /proc/$P/cmdline)"} # OK > CL=${(0A)$(< /proc/$P/cmdline)} # out of memory error I didn't reproduce the out-of-memory error with zsh-5.9-31, but I did reproduce an infinite loop in wordcount() because skipwsep() does not consider a metafied NUL byte to be whitespace. spacesplit() is entered with e.g. 'zsh\203 -f\203 ' and calls wordcount() on the first \203, which returns 2, then calls skipwsep() which returns without skipping anything because iwsep(' '^32) is false, and around we go again. I think the OOM is related because spacesplit() will allocate memory for each word that it believes it found, and the loop causes it to keep finding empty words forever. With sufficient RAM it just takes so long to use it all up that I give up waiting and kill the shell with gdb, which is how I found the loop case. However, I can't come up with a minimal test case to invoke the initial condition. It's not enough just to do e.g. ${(0A)$(</proc/$$/cmdline)} in isolation. Quotes matter because readoutput() does not call spacesplit() when $(...) is quoted, but I don't know why extendedglob would make any difference. > But even so, should zsh ever fail with an 'out of memory' error? There's really no way for the shell to know that it is imminently going to run out of memory and no useful way to recover once it has done so. Except in cases of bugs (which this appears to be), OOM only results from a programming error on the part of the user. Back to the original problem ... it appears that findsep(&s, NULL, 0) considers '\203 ' to be a separator (because a null byte is in $IFS ?) but skipwsep() does not. Can anyone else see why this doesn't break in every case of splitting strings with embedded NUL ?
[-- Attachment #1: Type: text/plain, Size: 1811 bytes --] On Fri, Aug 12, 2022 at 1:18 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > > However, I can't come up with a minimal test case to invoke the > initial condition. It's not enough just to do e.g. > ${(0A)$(</proc/$$/cmdline)} in isolation. > [...] it appears that findsep(&s, NULL, 0) > considers '\203 ' to be a separator (because a null byte is in $IFS ?) > but skipwsep() does not. OK, that's a red herring. The real problem is that itype_end(s, ISEP, 1) is not skipping over the '\203 ' pair in Jim's example, whereas it does in the simple example above. Once this becomes broken, it remains broken -- the simple example starts infinite-looping as well. It seems to come down to this in itype_end(): 4359 case ISEP: 4360 if (!wmemchr(ifs_wide.chars, wc, ifs_wide.len)) 4361 return (char *)ptr; On entry to that case in the simple example, wmemchr() returns nonzero for wc == 0. After edit-command-line, wmemchr() starts returning zero in that case. It appears ifs_wide has been erased. The problem starts here in edit-command-line: # Compute the cursor's position in bytes, not characters. setopt localoptions nomultibyte noksharrays When nomultibyte is set, inittyptab() is called and erases ifs_wide. This is not restored when emulation mode ends and multibyte is re-asserted. The following patch fixes this example, but might only be a partial fix for problems with locally flipping the state of various options (MONITOR, BANGHIST, SHINSTDIN come to mind). I think really we should be looping over the options and calling dosetopt() for each one instead of just memcpy'ing the saved set back on top of the original ... or at least we need a mapping of the subset of options that have extra code associated with a change via setopt. [-- Attachment #2: localnomultibyte.txt --] [-- Type: text/plain, Size: 790 bytes --] diff --git a/Src/exec.c b/Src/exec.c index f2911807c..77763f536 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5962,10 +5962,17 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) sticky = funcsave->sticky; } else if (isset(LOCALOPTIONS)) { /* restore all shell options except PRIVILEGED and RESTRICTED */ +#ifdef MULTIBYTE_SUPPORT + int flip_multibyte = opts[MULTIBYTE]; +#endif funcsave->opts[PRIVILEGED] = opts[PRIVILEGED]; funcsave->opts[RESTRICTED] = opts[RESTRICTED]; memcpy(opts, funcsave->opts, sizeof(opts)); emulation = funcsave->emulation; +#ifdef MULTIBYTE_SUPPORT + if (flip_multibyte != opts[MULTIBYTE]) + inittyptab(); +#endif } else { /* just restore a couple. */ opts[XTRACE] = funcsave->opts[XTRACE];
> 2022/08/13 9:10, Bart Schaefer <schaefer@brasslantern.com> wrote: > > The following patch fixes this example, but might only be a partial > fix for problems with locally flipping the state of various options > (MONITOR, BANGHIST, SHINSTDIN come to mind). I think really we should > be looping over the options and calling dosetopt() for each one > instead of just memcpy'ing the saved set back on top of the original > ... or at least we need a mapping of the subset of options that have > extra code associated with a change via setopt. In dosetopt(), only MONITOR, EMACSMODE/VIMODE and SUNKEYBOARDHACK have extra code other than new_opts[optno] = value. MONITOR: dosetopt() has the extra code only when setting MONITOR to ON. This suggests that just restoring opts[] is enough in doshfunc() when finishing a function, because: If MONITOR was initially off and was set to on locally in a function, no other code will be run even if we call dosetopt() to set it back to off. If MONITOR was initially on, then the extra code had been already run, and when it was set to off locally in a function nothing extra was done; so again just opts[MONITOR]=on is enough in doshfunc().... Is this correct? EMACS/VI: % bindkey -v % () { setopt localoptions emacs; } % (still using emacs binding) We need to restore the keymap in doshfunc() as in dosetopt(). The problem is that the options EMACS and VI can both be off. In doshfunc() we can't tell to which keymap we should link the 'main' keymap because both EMACS and VI may be off in the saved opts[]. The patch below forces 'bindkey -e/-v' to set the option EMACS or VI. Is this OK? SUNKEYBOARDHACK: this is simple. For inittyptab(): dosetopt() calls inittyptab() if BANGHIST or SHINSTDIN changes. With the patch below doshfunc() calls inittyptab() if BANGHIST, SHINSTDIN or MULTIBYTE changes (This should solve the original problem as Bart's patch). In addition, I think dosetopt() should call inittyptab() also when MULTIBYTE option changes. Otherwise, we will get troubles if MULTIBYTE option is changed not by the setopt builtin but by directly assigning to $options[multibyte]. (bin_setopt() always calls inittyptab(), but this may be redundant) I _hope_ there is no other option that require extra code in doshfunc(). diff --git a/Src/Zle/zle_keymap.c b/Src/Zle/zle_keymap.c index d90838f03..c0a12f71a 100644 --- a/Src/Zle/zle_keymap.c +++ b/Src/Zle/zle_keymap.c @@ -799,8 +799,11 @@ bin_bindkey(char *name, char **argv, Options ops, UNUSED(int func)) zwarnnam(name, "no such keymap `%s'", kmname); return 1; } - if(OPT_ISSET(ops,'e') || OPT_ISSET(ops,'v')) + if(OPT_ISSET(ops,'e') || OPT_ISSET(ops,'v')) { linkkeymap(km, "main", 0); + opts[OPT_ISSET(ops,'e') ? EMACSMODE : VIMODE] = 1; + opts[OPT_ISSET(ops,'e') ? VIMODE : EMACSMODE] = 0; + } } else { kmname = NULL; km = NULL; diff --git a/Src/exec.c b/Src/exec.c index f2911807c..6b70e8be9 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5961,11 +5961,27 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) emulation = funcsave->emulation; sticky = funcsave->sticky; } else if (isset(LOCALOPTIONS)) { + /* we need to call inittyptab() if these options change */ + int init_typtab = +#ifdef MULTIBYTE_SUPPORT + funcsave->opts[MULTIBYTE] != opts[MULTIBYTE] || +#endif + funcsave->opts[BANGHIST] != opts[BANGHIST] || + funcsave->opts[SHINSTDIN] != opts[SHINSTDIN]; + /* just restoring opts[] is not sufficient for these options */ + if (funcsave->opts[EMACSMODE] != opts[EMACSMODE] || + funcsave->opts[VIMODE] != opts[VIMODE]) + zleentry(ZLE_CMD_SET_KEYMAP, + funcsave->opts[VIMODE] ? VIMODE : EMACSMODE); + if (funcsave->opts[SUNKEYBOARDHACK] != opts[SUNKEYBOARDHACK]) + keyboardhackchar = funcsave->opts[SUNKEYBOARDHACK] ? '`' : '\0'; /* restore all shell options except PRIVILEGED and RESTRICTED */ funcsave->opts[PRIVILEGED] = opts[PRIVILEGED]; funcsave->opts[RESTRICTED] = opts[RESTRICTED]; memcpy(opts, funcsave->opts, sizeof(opts)); emulation = funcsave->emulation; + if (init_typtab) + inittyptab(); } else { /* just restore a couple. */ opts[XTRACE] = funcsave->opts[XTRACE]; diff --git a/Src/options.c b/Src/options.c index a1fe918fc..a994b563e 100644 --- a/Src/options.c +++ b/Src/options.c @@ -904,7 +904,11 @@ dosetopt(int optno, int value, int force, char *new_opts) keyboardhackchar = (value ? '`' : '\0'); } new_opts[optno] = value; - if (optno == BANGHIST || optno == SHINSTDIN) + if ( +#ifdef MULTIBYTE_SUPPORT + optno == MULTIBYTE || +#endif + optno == BANGHIST || optno == SHINSTDIN) inittyptab(); return 0; }
On Tue, Oct 18, 2022 at 8:08 AM Jun. T <takimoto-j@kba.biglobe.ne.jp> wrote: > > dosetopt() has the extra code only when setting MONITOR to ON. > so again just opts[MONITOR]=on is enough in doshfunc().... Is this correct? Hm, the shell having acquired the process group when MONITOR turned ON ... I don't know of any way to relinquish it again. So either this has to change in dosetopt() as well if it's even possible, or doshfunc() is OK. > The patch below forces 'bindkey -e/-v' to set the option EMACS or VI. > Is this OK? The only possible reason I can think of why this would not be OK is that it will change the output of "setopt" when its "form is chosen so as to minimize the differences from the default options for the current emulation". That seems a pretty insignificant issue, but might warrant mention in NEWS or even in the incompatibilities section of README. > I _hope_ there is no other option that require extra code in doshfunc(). The only ones I was worried about seem to be in the set of those that can't be changed after startup, or that can't change back again (PRIVILEGED, RESTRICTED).
> 2022/10/19 0:07, Jun. T <takimoto-j@kba.biglobe.ne.jp> wrote:
>
> EMACS/VI:
> % bindkey -v
> % () { setopt localoptions emacs; }
> % (still using emacs binding)
> We need to restore the keymap in doshfunc() as in dosetopt().
Sorry, I reconsidered this.
zle builtin has an option '-K keymap' to temporarily change the
keymap. I think users need not (or should not) use
'setopt localoptions emacs'.
Use of EMACS/VI should be highly discouraged, since turning them off
has no effect, and they are meaningless if a user-defined keymap
is in use.
Moreover, even with my patch, the following does not work:
% export VISUAL=vim
% zsh -f
% (vi keymap, but VI is not set)
% () { setopt localoptions emacs; ... }
% (now emacs keymap)
Instead of fixing this problem, I feel it is better to do
nothing for 'setopt localoptions emacs'. Then keymap is not
restored, but we can just say "please use zle -K".
Maybe we can add some more comments to the description of
EMACS and VI options in zshoptions(1), indicating that
use of these options is _highly_ discouraged.
Or should we try to restore the keymap?
> 2022/10/19 18:12, Jun T <takimoto-j@kba.biglobe.ne.jp> wrote: > > Instead of fixing this problem, I feel it is better to do > nothing for 'setopt localoptions emacs'. Here is a revised patch. Do nothing for EMACS/VI options, and add a few comments in the description of these options in options.yo. diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo index bf73664c9..445052617 100644 --- a/Doc/Zsh/options.yo +++ b/Doc/Zsh/options.yo @@ -2550,10 +2550,12 @@ pindex(NO_EMACS) pindex(NOEMACS) item(tt(EMACS))( If ZLE is loaded, turning on this option has the equivalent effect -of `tt(bindkey -e)'. In addition, the VI option is unset. +of `tt(bindkey -e)'. In addition, the tt(VI) option is unset. Turning it off has no effect. The option setting is -not guaranteed to reflect the current keymap. This option is -provided for compatibility; tt(bindkey) is the recommended interface. +not guaranteed to reflect the current keymap, and the tt(LOCALOPTIONS) +option does not work correctly. This option is provided only for +compatibility, and its use is highly discouraged. tt(bindkey) is the +recommended interface. ) pindex(OVERSTRIKE) pindex(NO_OVERSTRIKE) @@ -2582,10 +2584,12 @@ pindex(NO_VI) pindex(NOVI) item(tt(VI))( If ZLE is loaded, turning on this option has the equivalent effect -of `tt(bindkey -v)'. In addition, the EMACS option is unset. +of `tt(bindkey -v)'. In addition, the tt(EMACS) option is unset. Turning it off has no effect. The option setting is -not guaranteed to reflect the current keymap. This option is -provided for compatibility; tt(bindkey) is the recommended interface. +not guaranteed to reflect the current keymap, and the tt(LOCALOPTIONS) +option does not work correctly. This option is provided only for +compatibility, and its use is highly discouraged. tt(bindkey) is the +recommended interface. ) pindex(ZLE) pindex(NO_ZLE) diff --git a/Src/exec.c b/Src/exec.c index f2911807c..c8bcf4ee5 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5961,11 +5961,23 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) emulation = funcsave->emulation; sticky = funcsave->sticky; } else if (isset(LOCALOPTIONS)) { + /* we need to call inittyptab() if these options change */ + int init_typtab = +#ifdef MULTIBYTE_SUPPORT + funcsave->opts[MULTIBYTE] != opts[MULTIBYTE] || +#endif + funcsave->opts[BANGHIST] != opts[BANGHIST] || + funcsave->opts[SHINSTDIN] != opts[SHINSTDIN]; + /* take care of SUNKEYBOARDHACK but not of EMACS/VI */ + if (funcsave->opts[SUNKEYBOARDHACK] != opts[SUNKEYBOARDHACK]) + keyboardhackchar = funcsave->opts[SUNKEYBOARDHACK] ? '`' : '\0'; /* restore all shell options except PRIVILEGED and RESTRICTED */ funcsave->opts[PRIVILEGED] = opts[PRIVILEGED]; funcsave->opts[RESTRICTED] = opts[RESTRICTED]; memcpy(opts, funcsave->opts, sizeof(opts)); emulation = funcsave->emulation; + if (init_typtab) + inittyptab(); } else { /* just restore a couple. */ opts[XTRACE] = funcsave->opts[XTRACE]; diff --git a/Src/options.c b/Src/options.c index a1fe918fc..a994b563e 100644 --- a/Src/options.c +++ b/Src/options.c @@ -904,7 +904,11 @@ dosetopt(int optno, int value, int force, char *new_opts) keyboardhackchar = (value ? '`' : '\0'); } new_opts[optno] = value; - if (optno == BANGHIST || optno == SHINSTDIN) + if ( +#ifdef MULTIBYTE_SUPPORT + optno == MULTIBYTE || +#endif + optno == BANGHIST || optno == SHINSTDIN) inittyptab(); return 0; }