Ticket #4576 (closed defect: fixed)

Opened 4 months ago

Last modified 4 months ago

glitches in editor

Reported by: dimich Owned by: zaytsev
Priority: major Milestone: 4.8.33
Component: mcedit Version: 4.8.32
Keywords: Cc: dimich.dmb@…
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

Spurous characters appear in editor when open certain files with F4 (mc-xterm-F4.png).

The issue is reproduced in xterm, urxvt (mc-urxvt-F4.png) and virtual console, regardless of syntax highlight enabled or disabled.

Sometimes glitches remain after exit from editor (mc-xterm-F4-exit.png).

Redrawing screen by Ctrl+L removes glitches.

Example file to reproduce the issue attached.

When mcedit run from command line, sometimes top status line is not shown (mcedit-xterm.png). Ctrl+L forces top status line to display.

The issue is not reproduced in mc-4.8.31.

Attachments

mc-xterm-F4.png (21.0 KB) - added by dimich 4 months ago.
Edit file with F4 in xterm
mc-xterm-F4-exit.png (20.0 KB) - added by dimich 4 months ago.
Glitches remain after exit from editor
mc-urxvt-F4.png (15.2 KB) - added by dimich 4 months ago.
Glitches in urxvt
mcedit-xterm.png (17.3 KB) - added by dimich 4 months ago.
Top status line not shown
firmware.c (1.2 KB) - added by dimich 4 months ago.
Example file
0001-Ticket-4576-fix-visual-glitches-by-avoiding-g_module.patch (1.1 KB) - added by zaytsev 4 months ago.

Change History

Changed 4 months ago by dimich

Edit file with F4 in xterm

Changed 4 months ago by dimich

Glitches remain after exit from editor

Changed 4 months ago by dimich

Glitches in urxvt

Changed 4 months ago by dimich

Top status line not shown

Changed 4 months ago by dimich

Example file

comment:1 Changed 4 months ago by dimich

  • Cc dimich.dmb@… added

comment:2 Changed 4 months ago by andrew_b

I'm unable to reproduce this.

comment:3 Changed 4 months ago by zaytsev

  • Status changed from new to closed
  • Resolution set to worksforme

You did read how to create a defect report, right? No OS, no locale, no mc information. There is no way to debug your problem.

comment:4 Changed 4 months ago by andrew_b

  • Milestone Future Releases deleted

comment:5 Changed 4 months ago by dimich

OS: Arch Linux, kernel 6.10.6-arch1-1, glibc 2.40.

mc version is specified in the ticket: Version: 4.8.32

The issue is reproduced with default locale, i.e.:
$ locale
LANG=C
LC_CTYPE="C"
LC_NUMERIC="C"
LC_TIME="C"
LC_COLLATE="C"
LC_MONETARY="C"
LC_MESSAGES="C"
LC_PAPER="C"
LC_NAME="C"
LC_ADDRESS="C"
LC_TELEPHONE="C"
LC_MEASUREMENT="C"
LC_IDENTIFICATION="C"
LC_ALL=

The issue is reproduced without any custom configuration, i.e. /etc/mc and ~/.config/mc removed.

It is NOT reproduced the same way under different users. But once glitches appear in mc instance running under one user, they also appear in another instance running under another user.

It is NOT reproduced in mc-4.8.31.

I cannot reproduce the issue on another machine with the same OS, windom manager and other settings. Relevant difference between systems may be that one where i observe the issue has integrated Intel graphics, another one where i can't reproduce it has discrette Nvidia graphics. I have no idea how this difference may affect.

Let me know if any additinal information required.

comment:6 Changed 4 months ago by andrew_b

Since it is not reproduced in mc-4.8.31 and stable reproduced in mc-4.8.32 for you, git bisect is the way to find a bad commit:

git bisect start
git bisect bad 4.8.32
git bisect good 4.8.31

Then compile, test and mark the current commit as
git bisect bad
or
git bisect good.

comment:7 Changed 4 months ago by zaytsev

I think the problem might be with the C locale. I've seen glitches before when viewing binary files or editing some files with funny stuff in them. In theory we should not output any unprintable characters in single byte mode, but I never felt like it was worth dealing that stuff. Can you try with a clean UTF-8 locale and see if you get any glitches to confirm that?

Andrew, what do you think? Are glitches in single-byte locales a feature or a bug in your opinion?

comment:8 Changed 4 months ago by dimich

Seems I found the root cause of the issue.
In Arch mc is built with aspell support (./configure ... --enable-aspell ...) but i have no aspell package installed. If aspell libraries are installed the issue is not reproduced, and vice versa.
Looks like inablility to load aspell libraries leads to some kind of undefined behavior.
It's up to you to decide whether this is a mc bug or not.

comment:9 Changed 4 months ago by andrew_b

Replying to zaytsev:

I think the problem might be with the C locale.

Probably not with C locale only but with single-byte one. But in my case (KOI8-R) there is no such glithes.

I've seen glitches before when viewing binary files or editing some files with funny stuff in them. In theory we should not output any unprintable characters in single byte mode, but I never felt like it was worth dealing that stuff. Can you try with a clean UTF-8 locale and see if you get any glitches to confirm that?

Andrew, what do you think? Are glitches in single-byte locales a feature or a bug in your opinion?

Apparently, it's not good: #2180, #2453.

comment:10 follow-up: ↓ 11 Changed 4 months ago by zaytsev

Looks like inablility to load aspell libraries leads to some kind of undefined behavior.

So it's valid for both 4.8.31 and 4.8.32?

This is very strange, the only fishy thing I can see is the call of g_module_close on NULL. Does the following help?

diff --git a/src/editor/spell.c b/src/editor/spell.c
index 1f9aeba60..2dcbe0027 100644
--- a/src/editor/spell.c
+++ b/src/editor/spell.c
@@ -174,6 +174,9 @@ spell_available (void)
 
     spell_module = g_module_open ("libaspell", G_MODULE_BIND_LAZY);
 
+    if (spell_module == NULL)
+        return FALSE;
+
     if (spell_module != NULL
         && ASPELL_FUNCTION_AVAILABLE (new_aspell_config)
         && ASPELL_FUNCTION_AVAILABLE (aspell_dict_info_list_elements)

comment:11 in reply to: ↑ 10 Changed 4 months ago by dimich

So it's valid for both 4.8.31 and 4.8.32?

I see no any malfunction with 4.8.31 when aspell is not installed, only with 4.8.32. ./configure options are the same for both versions.

This is very strange, the only fishy thing I can see is the call of g_module_close on NULL. Does the following help?
+ if (spell_module == NULL)
+ return FALSE;

Yes, the patch seems to fix the issue. At least it is not reproduced in the same environment.

As I can see with gdb, g_module_close() does a lot even if module==NULL. Looks like it tries to set an error code.

I have glib2 2.80.4 installed, if that matters.

comment:12 follow-up: ↓ 13 Changed 4 months ago by zaytsev

  • Status changed from closed to reopened
  • Resolution worksforme deleted
  • Milestone set to 4.8.33

I see no any malfunction with 4.8.31 when aspell is not installed, only with 4.8.32.

Ok, thank you, maybe it is related to 0749b6d2d3352805691d1e38e8c1b7daf230a05d - could be that g_module_close does something it didn't do before.

Yes, the patch seems to fix the issue. At least it is not reproduced in the same environment.

Sounds good, thanks for reporting and testing. Andrew are you ok for me to commit?

comment:13 in reply to: ↑ 12 Changed 4 months ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from no branch to approved

Replying to zaytsev:

Andrew are you ok for me to commit?

Yes, please.

comment:14 Changed 4 months ago by zaytsev

  • Status changed from reopened to accepted
  • Owner set to zaytsev
  • Votes for changeset changed from andrew_b to committed-master
  • Branch state changed from approved to merged

comment:15 Changed 4 months ago by zaytsev

  • Status changed from accepted to testing
  • Resolution set to fixed

comment:16 Changed 4 months ago by andrew_b

  • Status changed from testing to closed

comment:17 Changed 4 months ago by dimich

Thank you.

FYI, git bisect found out that this commit introduced the issue: 822ef80f5b5da035ae41f8d0940cbc51293814f9

Last edited 4 months ago by dimich (previous) (diff)

comment:18 Changed 4 months ago by zaytsev

That makes sense, so we did it before, but it was lost upon refactoring, thanks!

Note: See TracTickets for help on using tickets.