We have received at least 5 bugs for WorldEditor that seem to have similar characteristics
- Inexplicable crash to desktop with no error message
- Happening on Windows 10, not reproduceable on Windows 7, Linux, or WINE
- Happens shortly after using the Truck Destination tool
In short, until we get a new beta out that officially fixes the issue, try downloading this nightly build of WED. Also, see all the bugs that are marked as duplicate and related to WED-780.
What happened?
Windows Tool Tips and You
Tool tips are those little pop ups that appear when you hover your mouse over UI elements. Sometimes their text includes helpful tips, sometimes they include data; it is whatever we program them to have.
The code to do that is located in xptools\src\GUI\GUI_Window.cpp:1252-1272. Windows passes a message to WED saying the user has hovered over some UI element, we hand over some text to a data structure (NMTTDISPINFO), Windows uses the data to display the tool tip.
Look at line 1272. This is the line that does the copying our tip string into the szText for Windows to display.
//wcs = wide-char-string = Unicode string, cpy means copy, _s means "more secure" version of the function wcscpy_s( di->szText, //The data in here will show up in 80, //Our stupid magical number convert_str_to_utf16(tip).c_str() //convert our UTF-8 string to UTF-16 to please Bill Gates );
This line was the source of the crashes.
What’s so special about the number 80 and “_s”?
Window’s gives you a free “no-work” string, szText, for your tool tip text that is 80 characters long. If you want a longer tool tip, you’ll have to do a bunch of (not well documented) work.
What makes this a “secure” string function is it immediately ends the program if you attempt to copy more than the specified 80 characters into the new string. This information is overwhelmingly under-documented. In addition, the crash doesn’t included any information about the cause, even in debug mode!
How did this go on for so long without getting caught?
Amazingly, we’ve never really had this be a problem, or previous versions of Windows have had this function not crash. One reason why this is now getting reported is due to the Truck Destination tool’s Truck Types property. When you add up enough of those strings and hover your mouse over them, it quickly ends up being much longer than 80 characters.
"Baggage Loader" + "Baggage Train" + "Fuel Truck (Jets)" + ""Fuel Truck (Liners)" + "Fuel Truck (Props)" = 81 characters in total.
- It is also possible we’ve just been getting lucky with our memory, always having strings getting corrupted exactly just right.
- It is also possible that all our data is short, with abbreviations being so common in aviation, its normally hard to reach more than 80 characters.
- Maybe no one reported a bug (please report your problems so we can fix them!)
Please comment if you have some ideas of why this doesn’t seem to happen on Windows 7, but does on Windows 10. Maybe this “secure” function isn’t as secure as we all think it is.
The solution: truncation!
Our easy fix for this is to chop a string off at 77 characters and append a “…” at the end. Hopefully no-one will mind this.
This new fix will soon make it into a beta and a release. Until then we have this nightly build available.
Final Thoughts
Thanks to a video sent in by a user, this was a pretty easy WED bug to find and solve. The hard part was weeding through all the bug reports that seemed shrouded in unrelated but related mystery. No one was realizing their most trusted friend, the mouse and common tool-tip, were causing the problem.
If you reported a bug, and this helps, please comment and contact us so we can clean out all the mysterious non-mysteries and get back to triaging bugs that have not been fixed!
Thank you for your patience as always as we try our best to make WED the best we can, and thank you to the people who report their problems instead of suffering through it, making a work around, or assuming their simply doing it wrong: You make the product better for everyone, including the developers!
> convert our UTF-8 string to UTF-16 to please Bill Gates
As a Unix guy I can appreciate the humor in this and I share your frustration with string handling on Windows. It’s a crazy hodge-podge of nonsense all over the place. Each and every time I try to do anything string-related on Windows, I feel like I’m stumbling into bizzarro-land of weird TCHAR *, multibyte string conversions, functions taking crazy complex arguments and the resulting undocumented behaviors if you happen to come up with an argument mix the author obviously didn’t anticipate. It’s like they picked up a book on designing clean APIs and said to themselves “yeah, let’s do the exact opposite of that”.
Ted Greene? You must be the new guy? Thanks for fixing this bug in Wed 1.6b1 🙂 Hopefully, we’ll get the green light to start uploading our 1.6 airports to the Gateway soon 🙂
Might want to retitle or lead this post to mention that the trouble is in WED, not X-Plane.
Hey Ben,
I can report that based on the night build I cannot crash WED the way I reported before.
Shouldn’t the 80 in wcscpy_s have limited your text copy to no more than 80 characters?
Perhaps the real problem is that when the copied text is 80 or more characters long, then the terminating null character is omitted and Windows merely thinks that the string is longer than 80 because it doesn’t find the terminating null character within the specified number of bytes
Please let me know if you agree that my comment is correct.
You would have thought that. Unfortunately it appears that the secure version requires the length of the source string to be less than or *exactly* equal to the same as the number of elements (80) you pass in.
Precisely. Hence the solution, if a string is too long, chop off the end and replace it with “…”, ensuring an exact length is 80.
Yes, but you can only put in 79 characters. The 80 character should be null.
Your string should probably be null terminated.
“Window’s gives you a free “no-work” string, szText, for your tool tip text that is 80 characters long. If you want a longer tool tip, you’ll have to do a bunch of (not well documented) work.”
Hmm… its very easy to pass a pointer to a longer string. Just create a buffer that is large enough and assign the pointer of that buffer to szText.
It is indeed easy to allocate a string, but the question is where should we put it and how long do we have to keep it? Until Windows decides? Until the tooltip goes away? When do we get told about that? Does the allocation have to be a special Win32 allocation? In WED, what part of the code base should this edge feature get stuck into?
In (briefly) looking for documentation about this, we didn’t find many answers to those questions. We want to keep things tidy and memory leak free, and this seemed like a great way to not accomplish that.
This was an architectural decision, a lack of confidence decision. The benefit of having a tool tip that can be longer than 80 chars is outweighed by the potential cost of this. So, if people really really want that, make a feature request saying why and we’ll start re-thinking it!
We also accept pull requests so if you’re familiar with it, please jump in. Thanks for asking good questions!
“What makes this a “secure” string function is it immediately ends the program if you attempt to copy more than the specified 80 characters into the new string.”
More exactly in the case of the parameter error “secure” function calls one of _invalid_parameter, _invalid_parameter_noinfo, or _invalid_parameter_noinfo_noreturn error handlers (depends on situation). And if you don’t like them, you may set your own error handler using _set_invalid_parameter_handler.
Oh my, the code is scary. Wild mix of Qt, WinAPI and plain C? Why?…
You mean WED?
– Qt is used for the Linux abstraction of the OS and window manager.
– Win32 is used for the Win32 abstraction of the OS and window manager.
– ObjC/Cocoa is used for the OS X abstraction of the OS and window manager. How could you LEAVE THIS OFF in your list of scary – it’s the most scary part of all. 🙂
– The plain C is legacy from a million years ago when I thought it would be useful for some low level utility APIs to be pure C so they could be compiled into a wider range of projects.
Since no one really uses that last capability we now have a policy of rewriting to C++ and removing C string handling whenever possible, to minimize C string face-palms. But the boundary with the Win32 API is one of those areas where we just have to do it right.
Why use Qt only for Linux? It solves the crossplatform problem beautifully.
I don’t usually use the term “beautifully” with anything Qt related. 🙂
Since Qt is already an abstraction for all OS and window managers (Linux, Windows, OS X), why not use it for all and drop your own abstractions?
There were a few reasons:
1. We wrote our abstraction first, then others brought in QT, so by the time it was available the work was already done.
2. I am not a huge QT fan, and wasn’t looking to have the QT tool chain on Mac/Windows.
Please clarify, are you talking about the X-Plane API being rewritten in C++? I really kind of enjoyed it being in C, since I’m using it from a C plugin.
No. The X-Plane SDK API bindings will remain pure C, even though the underlying implementation has always been C++ internally; we do this so that other languages can link more easily.
I am referring to trying to keep small parts of the scenery tools/WED code base C-only. We’re going to accept that scenery tools is all c++ and replace the C string handling.
Thank you.
On the truncation solution: I had a similar problem when creating audio file names from the track title, and I solved it similarly, but a bit different: First I decided to truncate the middle part, because the “ends” may have relevant data. For example the track title “Beethoven: Symphony No. 3 ‘Eroica’, op. 55 – Marcia funebre (Adagio assai)” became file name “4.02 Beethoven- Symphony No. 3 ‘Eroica’,…ebre (Adagio assai).ogg”. This shows the second improvement: I was worried wasting three characters for the ellipsis, so I used _one_ Unicode character that contains the three dots, saving two valuable characters 😉
Note that cutting out individual codepoints – or even worse bytes – from a string can cause the string to corrupt. Strangs are made out of graphemes, which can each be made out of multiple unicode codepoints, which can each be made out of multiple unicode surrogate codepoints, which can each be made out of multiple bytes. The wide string functions from the C (and C++) runtime library do not apply a normalization when transforming the input, and therefore will output surrogate codepoints if surrogates were used in the input, which is very common.
Attempting to print a string that contains broken unicode can cause undefined behaviour due to bugs in the implementations: Over here, I have Qt misrendering the complete string, Java same, Win32 leaking memory and misrendering all characters past the broken one and pangocairo entering an infinite loop on a number of sample inputs the other day.
Instead, if a string needs modification, it has to be modified on grapheme level.