Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
molsonkiko opened this issue Jan 22, 2024 · 6 comments · Fixed by #88
Closed
Labels
bug Something isn't working

Comments

@molsonkiko
Copy link
Contributor

molsonkiko commented Jan 22, 2024

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
Copy link

santropedro commented Feb 13, 2024

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
Copy link
Contributor Author

molsonkiko commented Feb 14, 2024

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
Copy link
Contributor

rdipardo commented Mar 2, 2024

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
Copy link
Contributor Author

molsonkiko commented Mar 2, 2024

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 BdR76 added the bug Something isn't working label Mar 9, 2024
@BdR76 BdR76 closed this as completed in #88 Mar 10, 2024
@BdR76
Copy link
Owner

BdR76 commented Mar 10, 2024

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

@molsonkiko
Copy link
Contributor Author

molsonkiko commented Mar 10, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants