nc-ng in some circumstances doesn't open requested documents in tabs

[eteran]

Getting warmer, but that doesn't fully explain this:

woz$ ls src/*.h | wc -l
     104
woz$ nc-ng src/*.h       # Just "Untitled"
Socket Error State: 1
Socket Error String: QLocalSocket: Remote closed

woz$ ls src/*.cpp | wc -l
      69
woz$ nc-ng src/*.cpp     # Just "Untitled"
Socket Error State: -1
Socket Error String: Unknown error

woz$ nc-ng client/nc.cpp # Opened properly
Socket Error State: -1
NEdit: client request packet received:
[
    {
        "create": 0,
        "geometry": "",
        "iconic": 0,
        "is_tabbed": -1,
        "langMode": "",
        "line_number": 0,
        "path": "/Users/anj/Software/other/nedit-ng/client/nc.cpp",
        "read": 0,
        "toDoCommand": "",
        "wait": false
    }
]

Socket Error String: Unknown error

I did just trigger these too, which are a bit different:

woz$ nc-ng src/*.h
QAbstractSocket::waitForBytesWritten() is not allowed in UnconnectedState
Socket Error State: 1
Socket Error String: QLocalSocket: Remote closed
woz$ nc-ng src/*.cpp
QAbstractSocket::waitForBytesWritten() is not allowed in UnconnectedState
Socket Error State: 1
Socket Error String: QLocalSocket: Remote closed

Originally posted by @anjohnson in #249 (comment)

[eteran]

This issue was brought up by @anjohnson in #249 . The core issue was two fold.

1. That nc-ng would fail for large sets of files (I believe fixed)
2. That nc-ng would open each file in a new window despite tab settings.

[anjohnson]

I don't think that first issue is completely fixed, sorry!

The tests below were using nedit-ng version 2020.1-147-g63130309 on MacOS Mojave.

Starting a server and trying to open 104 files in a single command gives me this:

woz$ nc-ng src/*.h
Warning: NEdit: error processing server request: [1] QLocalSocket: Remote closed

and just a single "Untitled" window opens.

When starting a server and opening 69 files though (nc-ng src/*.cpp) they all open up properly in a single window with no terminal window messages.

If the server is already running, asking for 69 files causes them all to open in separate windows (leaving the Untitled window also open) with no terminal messages. As those 69 windows open up each subsequent one seems to take longer than the previous one. The early ones aren't really noticable, but later windows seem to slow quite substantially with the last one taking almost a second to appear – not really a problem, but an indication of some kind of non-linearity in the code.

If the server is already running, asking for 104 files just results in the same "QLocalSocket: Remote closed" message as above in the nc-ng terminal window.

Thanks!

[eteran]

So, yea if you get an error on the command line, you'll only see an untitled window. That's because it launched the server, which defaults to showing an untitled window, and then fails to open any documents.

When the server is already running, is it on a separate desktop or anything like that? There is logic to try to find windows on the "current screen", but I'm wondering if it has flaws.

As for the "Remote closed", that's disappointing that it is still an issue, I'll have to keep looking into it.

Is there anything else you can tell me about your system that would help me reproduce the issue?

working over ssh forwarded? Or just directly in macOS? I am wondering if macOS has unusually small pipe buffer sizes or something that needs to be worked around :-/

[eteran]

@anjohnson Can you give this PR a try and see if it helps at all?

#255

I don't know if you have X11 libs installed on your mac, but if you do then the nedit server would be using that to try to find existing windows (and maybe failing?).

This makes it so on mac, it always using the "Qt way of doing things". I'd like to go pure Qt all the time, but there were other issues back in the day the first time I tried to do that :-P.

[anjohnson]

In that case it was just regular MacOS, no X11 or ssh forwarding involved.

I just built the same code on my RHEL-7.9 Linux system:

nedit-ng version 2020.1-147-g63130309

     Built on: Linux, x86_64, GCC 7.3.1 20180303 (Red Hat 7.3.1-5)
      With Qt: 5.9.7
   Running Qt: 5.9.7
       Locale: en
   Git branch: master
   Git commit: 63130309a1676cdfd860cec515f466c89aff1fdf

I'm connecting to that machine through NX, but to the applications it just looks like the local screen (DISPLAY is :0). There are some more (different) issues there related to the screen and fonts which I don't have time to describe right now, but running nc-ng src/*.h there succeeded (no messages) and gave me 3 windows; one regular properly-sized one (80x60 chars) with lots of tabs containing many of the files, another one with another set of tabs containing more files (but this window is only 75x18 chars in size), and a third with just one file also 75x18 chars in size (this holds the first file in the list, BlockDragTypes.h.)

So yes it may be that the Mac has a smaller pipe buffer, is there a way to request a specific size for the pipe backing store when you create it?

I don't have time to try your other branch right now but I will, maybe later tonight or tomorrow.

[anjohnson]

I just built this on MacOS:

nedit-ng version 2020.1-149-g9f50e446

     Built on: BSD4, x86_64, Clang 11.0 (Apple)
      With Qt: 5.15.1
   Running Qt: 5.15.1
       Locale: en
   Git branch: master
   Git commit: 9f50e446cf6b717fecd9ee9b23fa2a95e17b532b

Running nc-ng src/*.h succeeded in that it started the server and opened all the files in separate tabs belonging to the one window. Closing the window is reasonably quick.

Running nc-ng src/*.h with an already-running server opened all the files, each one was in a separate window and by the end of the list it was taking about a second to open each one. Closing all those windows again was also pretty slow, taking maybe 10 seconds or more (I didn't actually time it).

Completely unrelated, it's interesting to see how Mojave displays the drop shadow of 104 windows placed exactly on top of each other, normally you don't notice the granularity of the shading because it's so light:

Here are some more interesting and possibly relevant data points which seems fairly consistent: nc-ng; nc-ng src/*.h generally opens all the files in tabs of the server window that the first command starts, whereas if I run nc-ng to start the Untitled server and then separately run nc-ng src/*.h I get 104 separate windows (plus the Untitled window). If I start an Untitled server then separately run nc-ng; nc-ng src/*.h first a second "Untitled_1" window appears, then it opens the files in tabs inside both of the windows. Is that expected?

Note that Closing a window with 104 tabs is much faster than closing 104 windows.

Okay, I'll try the other branch now, separate comment.

[anjohnson]

Using this version:

nedit-ng version 2020.1-153-g610b0020

     Built on: BSD4, x86_64, Clang 11.0 (Apple)
      With Qt: 5.15.1
   Running Qt: 5.15.1
       Locale: en
   Git branch: HEAD
   Git commit: 610b002073e91bed39fd983b99726c59afc8171c

Hmm, the first time I ran this I first started an Untitled server, then (after pasting the above from its About box) I ran my usual nc-ng src/*.h. The window immediately disappeared, and a few seconds later an Apple dialog "nedit-ng quit unexpectedly" popped up, with a Report button which causes a nice Problem Report window to appear, from which I have extracted the following crash data:

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000008
Exception Note:        EXC_CORPSE_NOTIFY

Termination Signal:    Segmentation fault: 11
Termination Reason:    Namespace SIGNAL, Code 0xb
Terminating Process:   exc handler [19088]

VM Regions Near 0x8:
--> 
    __TEXT                 000000010fe4a000-0000000110055000 [ 2092K] r-x/r-x SM=COW  /Users/USER/*

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   org.qt-project.QtWidgets      	0x0000000110521754 QWidget::windowHandle() const + 4
1   nedit-ng                      	0x000000010ff128a4 NeditServer::newConnection() + 292 (NeditServer.cpp:39)
2   org.qt-project.QtCore         	0x00000001112d34e2 0x1110af000 + 2245858
3   org.qt-project.QtNetwork      	0x0000000110282df7 QLocalServer::incomingConnection(unsigned long long) + 327
4   org.qt-project.QtNetwork      	0x0000000110285f04 0x1101c3000 + 798468
5   org.qt-project.QtCore         	0x00000001112d362c 0x1110af000 + 2246188
6   org.qt-project.QtCore         	0x00000001112dbdaa QSocketNotifier::event(QEvent*) + 602
7   org.qt-project.QtWidgets      	0x00000001104fcadd QApplicationPrivate::notify_helper(QObject*, QEvent*) + 269
8   org.qt-project.QtWidgets      	0x00000001104fdea2 QApplication::notify(QObject*, QEvent*) + 466
9   org.qt-project.QtCore         	0x00000001112a1274 QCoreApplication::notifyInternal2(QObject*, QEvent*) + 212
10  org.qt-project.QtCore         	0x00000001112f6317 0x1110af000 + 2388759
11  com.apple.CoreFoundation      	0x00007fff2ef39b63 __CFSocketPerformV0 + 976
12  com.apple.CoreFoundation      	0x00007fff2eea09fb __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
13  com.apple.CoreFoundation      	0x00007fff2eea09a1 __CFRunLoopDoSource0 + 108
14  com.apple.CoreFoundation      	0x00007fff2ee847df __CFRunLoopDoSources0 + 195
15  com.apple.CoreFoundation      	0x00007fff2ee83da7 __CFRunLoopRun + 1196
16  com.apple.CoreFoundation      	0x00007fff2ee836a9 CFRunLoopRunSpecific + 459
17  com.apple.HIToolbox           	0x00007fff2e0d41ab RunCurrentEventLoopInMode + 292
18  com.apple.HIToolbox           	0x00007fff2e0d3ee5 ReceiveNextEventCommon + 603
19  com.apple.HIToolbox           	0x00007fff2e0d3c76 _BlockUntilNextEventMatchingListInModeWithFilter + 64
20  com.apple.AppKit              	0x00007fff2c46b77d _DPSNextEvent + 1135
21  com.apple.AppKit              	0x00007fff2c46a46b -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1361
22  com.apple.AppKit              	0x00007fff2c464588 -[NSApplication run] + 699
23  libqcocoa.dylib               	0x0000000113c8e88c 0x113c56000 + 231564
24  org.qt-project.QtCore         	0x000000011129d3ff QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) + 431
25  org.qt-project.QtCore         	0x00000001112a1802 QCoreApplication::exec() + 130
26  nedit-ng                      	0x000000010ff83855 main + 1461
27  libdyld.dylib                 	0x00007fff5aec83d5 start + 1

Thread 1:: com.apple.NSEventThread
0   libsystem_kernel.dylib        	0x00007fff5affd21a mach_msg_trap + 10
1   libsystem_kernel.dylib        	0x00007fff5affd768 mach_msg + 60
2   com.apple.CoreFoundation      	0x00007fff2ee849e1 __CFRunLoopServiceMachPort + 327
3   com.apple.CoreFoundation      	0x00007fff2ee83f4e __CFRunLoopRun + 1619
4   com.apple.CoreFoundation      	0x00007fff2ee836a9 CFRunLoopRunSpecific + 459
5   com.apple.AppKit              	0x00007fff2c4734a2 _NSEventThread + 175
6   libsystem_pthread.dylib       	0x00007fff5b0bc2eb _pthread_body + 126
7   libsystem_pthread.dylib       	0x00007fff5b0bf249 _pthread_start + 66
8   libsystem_pthread.dylib       	0x00007fff5b0bb40d thread_start + 13

Thread 2:: com.apple.CFSocket.private
0   libsystem_kernel.dylib        	0x00007fff5b004616 __select + 10
1   com.apple.CoreFoundation      	0x00007fff2eeb1fc8 __CFSocketManager + 630
2   libsystem_pthread.dylib       	0x00007fff5b0bc2eb _pthread_body + 126
3   libsystem_pthread.dylib       	0x00007fff5b0bf249 _pthread_start + 66
4   libsystem_pthread.dylib       	0x00007fff5b0bb40d thread_start + 13

Thread 3:
0   libsystem_pthread.dylib       	0x00007fff5b0bb3f0 start_wqthread + 0

Thread 0 crashed with X86 Thread State (64-bit):
  rax: 0x0000000000000000  rbx: 0x00007ffedfdb32f0  rcx: 0x0000000000000000  rdx: 0x0000000000000400
  rdi: 0x0000000000000000  rsi: 0x00007ffedfdb3280  rbp: 0x00007ffedfdb32b0  rsp: 0x00007ffedfdb32b0
   r8: 0x00000000519de648   r9: 0x00000000fffffcff  r10: 0x00007fb519d00000  r11: 0x0000000000000010
  r12: 0x00007fb519a4d180  r13: 0x00007fb519d66540  r14: 0x0000000000000000  r15: 0x0000000000000001
  rip: 0x0000000110521754  rfl: 0x0000000000010202  cr2: 0x0000000000000008
  
Logical CPU:     0
Error Code:      0x00000004
Trap Number:     14

Actually it looks like any attempt to use nc-ng to connect to a server results in a the server crashing with the same stack-trace, irrespective of how many files are requested or whether the server has any files open or not.

With no server running, nc-ng src/*.h still does the right thing (as the master branch did described in my previous comment).

I could probably run this in a debugger if you want any more details, I do use VSCode.

[eteran]

Looks like a NULL pointer (access to an address bear zero usually means a bull pointer reference via a strict member)

I really need to setup a mac to test things!

In other news, I reworked in both this and master how how list is sent to the server.

Did that fix (in master) the issue of large lists failing?

[anjohnson]

In NeditServer.cpp line 208:

QScreen *current_desktop() {
	return screenFromWidget(QApplication::activeWindow());
}

QApplication::activeWindow() is returning NULL, which screenFromWidget() doesn't check for or handle.

[anjohnson]

Yes, I think your fix in 9f50e44 has solved the problem of passing long lists of files from nc-ng to the server.

I just realized that my issues have to be related to the fact that I'm using 2 screens on this laptop, the built-in one and a TV, and all my tests have been run from a terminal window on the TV. nc-ng normally creates the Untitled window on the same screen as the terminal. If I drag the Nedit-ng window to the built-in screen though, subsequently running nc-ng Settings/*.h successfully opens all the files in tabs in that window. Drag the window back to the TV and nc-ng README.md opens it in a new window. This is with the master branch version; your alternate-nc-desktop-detection branch still Segfaults even if the both the terminal and server windows are on the built-in screen.

[eteran]

@anjohnson Yea, I think I see what's going on here... maybe. I have an idea of how to fix it, but we'll see if it works. Basically, I think that the code to "find a window on the 'current desktop'" is always failing, even after creating a window.

So I think the solution is that if a window is created at all, then remember that one and use it as the target for future documents regardless of which desktop it happened to open on, because that one is by default the "current desktop" now.

And yea, fixing that NULL pointer should be easy. I guess Qt on macOS doesn't consider a window active until after it is made visible by the OS or something?

[anjohnson]

I suspect the issue is that the code to find what screen a window is on may be returning a different result at different times. You have multiple threads here, and this evidence that I found earlier should be explainable by whatever theory you come up with:

Here are some more interesting and possibly relevant data points which seems fairly consistent: nc-ng; nc-ng src/*.h generally opens all the files in tabs of the server window that the first command starts, whereas if I run nc-ng to start the Untitled server and then separately run nc-ng src/*.h I get 104 separate windows (plus the Untitled window).

Actually that description might be slightly wrong, the difference seems to be whether the focus belongs to the Untitled window when the server is looking for it. Using this: nc-ng; sleep 5; nc-ng README.md if I keep the focus in the Untitled window the README file gets opened in that window; If during the sleep I move the focus to a different window I get a new window with the README file. I can even click away and then back into Untitled and it finds the window.

[eteran]

@anjohnson Can you give master another go, I just made a small tweak in light of the fact that sometimes there isn't an "activeWindow" yet.

[anjohnson]

No change; if I drag the Untitled window to the built-in screen a subsequent nc-ng README.md opens it in that window, but if the Untitled window is on the TV where my terminal window is it opens in a new window. There is an active window in this case, but I suspect it isn't being used because the screen ID it's on doesn't match.

Feel free to send me a patch that prints out each window's screen identifier while it's looking through the list for the right window (and don't forget to also print what it's trying to match with)...

[eteran]

@anjohnson can you try this PR? #257 ?

It tries extra hard to use the same window for all documents opened by a single nc request.

[anjohnson]

The branch nc-window-pick doesn't help. On Mojave with 2 screens, using "nedit-ng version 2020.1-152-g6f93071a" from that branch and typing commands into an Apple Terminal window on my 2nd screen (TV):

$ nc-ng         # Create Untitled window on TV.
                # Move mouse & focus back to terminal, still on TV.
$ nc-ng src/*.h # Creates 104 windows on TV, each one slower to open than the previous.

Whereas:

$ nc-ng         # Create Untitled window on TV.
                # Drag Untitled window to laptop screen.
                # Move mouse & focus back to terminal on TV.
$ nc-ng src/*.h # Opens 104 tabs on what was the Untitled window on laptop screen.

and:

$ nc-ng         # Create Untitled window on TV.
                # Move mouse & focus back to terminal on TV.
$ sleep 5; nc-ng src/*.h # Click in Untitled window during sleep.
                # Opens 104 tabs on what was the Untitled window on TV.

Please concentrate on how NeditServer.cpp identifies which screen each window is displayed on, and which is the "current" screen. The server properly finds an Untitled window on the TV if it has the keyboard focus when nc-ng sends the request (which also means that a nedit-ng menu is displayed in Apple's top menu bar, but that is not where Qt is showing the regular File/Edit/Search etc menus, those appear per-window as normal). It also finds an Untitled window if it's on the first screen (laptop).

[anjohnson]

I just corrected the final example above, it should have read "Untitled window on TV"

[eteran]

@anjohnson that is VERY surprising. Because I added code specifically to say "whatever window you opened a document in, use that one again". So I was expecting it to at most create one new window with 104 tabs :-(.

I'll keep hacking at this though, I'm sure I'll get to the bottom of it eventually!

[eteran]

@anjohnson I think I'll have to whip out an old mac mini and see if I can replicate this locally. Unfortunately without being able to reproduce it, it's a challenge to know if I'm on the right track :-(.

[eteran]

@anjohnson First of all, sorry for the radio silence on this one, but it's been a bit of a crazy busy year. But I have good news!

@sjtringali tying you in since I have a "how should this work" kind of question at the end.

1. I have a mac to test this on :-)
2. I've replicated the issue (even on windows!)

I have a complete picture of what is happening:

Firstly, activeWindow() is indeed allowed to return NULL if no window is active, like if you are currently typing in a terminal, none of the nedit-ng windows are active.

Given that there isn't always an active window, it's not always easy to determine which screen the user should expect the new window to be on.

Finally, it gets a little quirky since (at least on windows), there is a concept of a "primary screen", which should be where new windows are expected to be created... but it seems to want to create them where the last window was.

So here's the deal. I need the nedit server to make a decision of "is there already a window on the screen where I want to make one" if so, make a new tab, if not, make a new window.

Sounds easy, but here's where it falls down:

When I place the first main window on screen 1 with a terminal on screen 0. Qt reports screen 0 as my primary, which makese. But then when I use nc-ng it creates the window on screen 1! Because nedit-ng said "OK, I the primary screen is 0, but the only window I have is on screen 1. So I should create a new window...

It gets worse!

When we open multiple files like this, it can create a new window for each one since that check fails over and over.

So on to my question of "what does the user expect".

I can PROBABLY track the last main window which had any focus at all, if it still exists, assume they want a new tab in that one.
I can DEFINITELY say "just always create new tabs in one of the existing windows, prioritizing based on age"
Could implement a different behavior I'm not currently thinking of too.

What are your thoughts?

[sjtringali]

I’ll have to try this out tomorrow to see. Most apps are pretty horrible at this anyway. Always using the primary screen is annoying, but, so is always using the screen that has your mouse cursor. First case is ignoring your perfectly valid case to group things together, and the second case, it’s annoying when you try to switch away from a screen in order to do something else, most often when something is taking a long time and blocking an application… And then a new window pops up from the old application on top of your current screen which you don’t want. So my answer is to try to design for both cases. Probably, you can store the last activated window. If there is no currently active window, then use the same screen as that one.

…

On Dec 14, 2021, at 10:49 PM, Evan Teran ***@***.***> wrote:  @anjohnson First of all, sorry for the radio silence on this one, but it's been a bit of a crazy busy year. But I have good news! @sjtringali tying you in since I have a "how should this work" kind of question at the end. I have a mac to test this on :-) I've replicated the issue (even on windows!) I have a complete picture of what is happening: Firstly, activeWindow() is indeed allowed to return NULL if no window is active, like if you are currently typing in a terminal, none of the nedit-ng windows are active. Given that there isn't always an active window, it's not always easy to determine which screen the user should expect the new window to be on. Finally, it gets a little quirky since (at least on windows), there is a concept of a "primary screen", which should be where new windows are expected to be created... but it seems to want to create them where the last window was. So here's the deal. I need the nedit server to make a decision of "is there already a window on the screen where I want to make one" if so, make a new tab, if not, make a new window. Sounds easy, but here's where it falls down: When I place the first main window on screen 1 with a terminal on screen 0. Qt reports screen 0 as my primary, which makese. But then when I use nc-ng it creates the window on screen 1! Because nedit-ng said "OK, I the primary screen is 0, but the only window I have is on screen 1. So I should create a new window... It gets worse! When we open multiple files like this, it can create a new window for each one since that check fails over and over. So on to my question of "what does the user expect". I can PROBABLY track the last main window which had any focus at all, if it still exists, assume they want a new tab in that one. I can DEFINITELY say "just always create new tabs in one of the existing windows, prioritizing based on age" Could implement a different behavior I'm not currently thinking of too. What are your thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[anjohnson]

That certainly seems to explain what's happening.

I think the most important thing is probably that if I ask nc-ng to open multiple files and my preference is set to "Open file in new tab" then all those files should open as tabs inside the same window, whether it's a new one or an existing one (see paragraph below). I don't care quite as much which screen gets used for new windows, but it would be nice to be able to control which existing window it would create new tabs on — preferably the last one that I had focussed (but since I'm running nc-ng in a terminal to open the files it won't be active at the time).

Supposedly the option -group should tell nc-ng to create one new window for the subsequent list of files but that doesn't work, presumably because of this bug. The first file in the list gets a new window, but all the subsequent files are opening in another window that I already had open on my primary desktop. If it worked -group would let me control whether to start a new window for this list of files.

Thanks!

[eteran]

@anjohnson OK, I think and hope that this PR will help with this.

#325

It does a few things:

• Uses portable QScreen API for simpler code.
• Uses the current cursor position to determine the "current screen"
• When possible, determines the preferred target once , and will reuse that window as needed instead of calculating it for each document (limiting possible changes for different documents to get a different window)

I know it's been a bit, but when you have a chance, can you see if this solves the issue for you since you've been best able to reproduce the problem?