03 Nov

30190

0

Two Steps from [the S]hell

Hello everyone, and sorry for the delay.

I caught a bad cold (that may or may not be complicated with a throat infection -- we'll see) so I was lacking the necessary concentration to write this report earlier.

As Z98 said last weekend, it has been some very busy days for all the team. I want to reiterate all the appreciation for the help given by everyone. There is just so much debugging I can do without my head exploding, and finding leaks tends to cross that line. :P

I believe this is for a similar reason to why I can’t play chess: I can either see the big picture, or the details, but not both at once. The more I focus on one single thing, the harder it is for me to keep track of all the rest of the system, and if I think of the system as a whole, I easily forget the important detail about the pieces of this system. This makes it very hard for me to track the usage of a little handle across all the network of COM objects that make use of it, and I can’t state enough how much I appreciate people helping me with such tasks.

I like to separate the leaks into three categories: high risk, low risk, and static leaks. High risk leaks are the ones that will inevitably cause the program (or computer) to stop working, and they will do so relatively fast. Low risk are leaks that would, in the long term, cause issues, but they grow slowly enough that you’d only notice them after weeks or months of uptime. Static leaks are leaks that happen during initialization of a program, and will never grow.

After the weekend, there were no more high risk nor low-risk leaks that we could see. And since static leaks are not really an issue, only a matter of cleaning up at shutdown, we could consider the leak hunt to be over.

At this point, we decided to do some deeper testing, and see which items in the TODO list would be next in the list, and how much remains until the shell can be merged into trunk. We quickly discovered a bug in the start menu, where clicking on the programs items wouldn’t actually launch the program.

Wanting to help more actively, Giannis restored his development environment and joined me in the coding effort (albeit only in his spare time). While Giannis worked on some smaller fixes, I started trying to fix the start menu issue.

The issue proved to be complex, involving multiple different bugs, related to the code flow involved in opening and closing shell menus. The issue hadn’t been noticeable until recently, because the COM object leaks allowed the information to remain present, but after fixing the leaks, the objects were destroyed too early, and the values needed to execute the items had been lost by the time the items tried to use it. Before the actual issue was found, though, I had to first fix the objects being destroyed prematurely.

I added some CComPtr references in key methods, which would keep a reference to their owner object and allow it to keep using the contents of the object until the method returned. When the code didn’t crash anymore, I learned that there was another underlying issue: even without crashes, the data was still lost.

I traced this to the change I made to the shell menus, so that they’d close before executing the item, which was needed for the Shutdown button. This change caused menus with content from an IShellFolder to lose the current item status between the time the click was received and the time the item was executed. In order to fix that, I had to save the data for later, so that it was available when it was needed later.

After this fix, I joined Giannis in fixing smaller issues and cleaning up the code. First, I fixed the View menu, so that the unimplemented items would appear grayed out, and also the toolbar items would reflect which toolbars are shown. I also implemented the handler for the items in the Favorites menu, which can now be properly launched when clicking on them, and I implemented the Close item in the File menu.

Amine left me a note informing me that the livecd builds weren’t working correctly. After a bit of debugging, I traced the problem to the folders used for the shell menus: the livecd has COMMON folders in “All Users”, but it lacks the user-specific menus in the user account. Because the shell wasn’t prepared for this situation, it would fail to initialize both the start menu and the Favorites menu. I fixed both, but it turns out the livecd doesn’t have a Favorites folder at all, so the problem remains that there is no Favorites menu to show. It should be fixed at some point so that the livecd contains an empty Favorites folder, or possibly one with some URL shortcuts for the website, forums and such.

I took this chance to change the “Is this copy legal?” item in the Help menu, so that it opens a web browser with the reactos FAQ page. If we ever have a better place that explains the legal status of the project and the legal ways to obtain installation images, we can always change that URL.

Annoyingly, it was while testing the livecd, that I noticed the start menu was still closing unexpectedly sometimes. After some debugging work, I realized that the closing was caused by another application becoming active. This shouldn’t have happened, but since the code that closed the menus on activation wasn’t necessary anymore, I chose to disable it. In the off-chance that it causes the menu to remain open in a situation that should have closed it, I’ll have to revisit this change but meanwhile, it’s best this way.

Work continued with polishing and code clean-ups, while fixing some small issues here and there. And while the shell is getting VERY close to merging, we decided to continue the polish work for a while longer, so that we have the best the shell has to offer, and not a rushed merge.

Until next weekend.

P.S.: Some interesting things happened over the weekend, but you will have to wait for next week’s report (or read the commit logs from the branch) to learn about them!

Discussion: https://www.reactos.org/forum/viewtopic.php?f=2&t=13735

This blog post represents the personal opinion of the author and is not representative of the position of the ReactOS Project.