BdR76 / CSVLint

CSV Lint plug-in for Notepad++ for syntax highlighting, csv validation, automatic column and datatype detecting, fixed width datasets, change datetime format, decimal separator, sort data, count unique values, convert to xml, json, sql etc. A plugin for data cleaning and working with messy data files.
GNU General Public License v3.0
151 stars 8 forks source link

Opening a dialog from the CSV Lint window causes NPP to hang if the window is undocked #83

Closed molsonkiko closed 4 months ago

molsonkiko commented 5 months ago

Steps to reproduce

  1. Open the CSV Lint window
  2. Undock it by clicking the top bar and dragging it until the ghost wireframe shows it floating.
  3. Try clicking one of the buttons that opens another dialog, like the Sort button.
  4. Notepad++ will hang and need to be closed manually.

image

I recognize that this is a relatively niche case, and I only found out about it in this discussion.

This happened while I was using 0.4.6.5 on Notepad++ 8.5.8.

As far as I can tell, JsonTools does not have a similar problem, and I don't really understand why. I've tried undocking the tree viewer and opening the Query to CSV and Find/replace dialogs from it while it is undocked, and I don't get a crash.

santropedro commented 5 months ago

I ran into the same issue. Thank god I didn't lost progress thank to autosave. I confirm your report 100%, in my case I tried "reformat","add column", "sort" and they always make it crash instantly (always to reproduce do it while undocked, docked I doesn't produce the crash). Task manager shows huge CPU activity too. bug admin DEBUG INFO: Notepad++ v8.6.2 (64-bit) Build time : Jan 14 2024 - 02:16:00 Path : C:\Program Files\Notepad++\notepad++.exe Command Line : Admin mode : OFF Local Conf mode : OFF Cloud Config : OFF OS Name : Windows 10 Home Single Language (64-bit) OS Version : 22H2 OS Build : 19045.3930 Current ANSI codepage : 1252 Plugins : ComparePlugin (2.0.2) CSVLint (0.4.6.6) mimeTools (3) NppConverter (4.5) NppExport (0.4)

molsonkiko commented 5 months ago

I tried to run this project in the Visual Studio debugger (with the target framework changed to 4.8 because I don't have Visual Studio 2019), and I was able to reproduce the bug.

With the debugger on, I observed that the infinite loop begins in the ShowDialog method of any child dialog. This is, inconveniently, not Bdr76's code. Since I found the bug when compiling with .NET Framework 4.8 (and I tested if an older version of JsonTools based on .NET Framework 4.0 had the bug, and it didn't), we can rule out framework-related issues.

When I turned on decompilation of C# core code, I found that the exact place where the infinite loop begins is in System.Windows.Forms.Application.ThreadContext.RunMessageLoopInner in a line that reads currentForm.Visible = true;

So for some reason, setting the Visible attribute of the dialog to true is causing an infinite loop. Obviously something else is happening behind the scenes, but the Visual Studio debugger can't dig any deeper than that, so I'm clueless as to what.

I also tried registering the CsvLintWindow with NPPM_MODELESSDIALOG to see if that would make any difference. It didn't help.

rdipardo commented 4 months ago

As a note to plugin developers facing this issue in the future, you should take the time to read this comprehensive discussion of how window message handling can go wrong in high-level languages like C# or Delphi/Object Pascal (the latter being the historical predecessor to .NET).

The TL;DR is — roughly — when you register a form with child components like buttons, memos, etc. (by sending NPPM_DMMREGASDCKDLG with the parent form's handle), the component hierarchy is altered by installing N++'s Docking Manager as the ultimate parent control. This means your form now "looks like" a child control to the operating system, and the form's own child controls are effectively invisible.

Problem is, your native GUI library (WinForms, VCL, etc.) doesn't know what has happened, so it keeps on sending window messages to your child controls. Since the OS believes your form is a child and not a parent, the UI thread gets caught in an endless loop of trying and failing to locate the child controls.

In my experience, the only effective workaround is to trap the WM_NOTIFY message with a custom window procedure and forcibly alter the component hierarchy. The original advice was to modify the form's extended window styles by clearing the WS_EX_CONTROLPARENT flag; in order words, clean up any stale attributes that your native GUI library may have set on the form before N++'s Docking Manager took over. More recent advice on this problem supports the opposite approach of setting the WS_EX_CONTROLPARENT flag instead, for example:

molsonkiko commented 4 months ago

Based on @rdipardo 's comment just now, I figured out what the problem is: the problem occurs when the button that opens another form is inside a GroupBox (or other "container" control; in CsvLint it's a SplitContainer) in a docking form.

The SelectionRememberingForm in NppCSharpPluginPack, which is a docking form, does not have this issue, but if I put the button that opens another form inside a group box, the problem re-emerges.

Note that having a GroupBox or other container in the form is not sufficient; if the button that opens another form is not inside a container, I can still open a form from the SelectionRememberingForm while it is undocked.

Once I figure out how to stop the bug in NppCSharpPluginPack, I'll submit a PR.

BdR76 commented 4 months ago

@molsonkiko Excellent work, thanks for the PR, this has fixed the issue

molsonkiko commented 4 months ago

I tried adding more or less the same code to my NppCSharpPluginPack, and it occasionally seems to cause Notepad++ to crash when I undocked a docking form (the SelectionRememberingForm).

I haven't replicated it the last few times I've tried, and I'm not even sure it's a problem with this plugin (the nondeterminism suggests it's related to multithreading, but this code is synchronous), but be on the lookout.

I cannot replicate the same problem with a local fork of CSVLint, which makes it even less likely that it's a problem with this code.