Want to protect your cyber security and still get fast solutions? Ask a secure question today.Go Premium

x
?
Solved

[Bugfix] Simple code for trayicons, please check...

Posted on 2004-11-29
19
Medium Priority
?
1,031 Views
Last Modified: 2010-04-05
Hi, Experts.

I created a nice and very simple piece of code for a trayicon interface to be used in very small applications. (It still depends on some VCL units but hey, a console application with this unit will still be smaller than 60 KB.)

I fear, however, that there are quite a few minor flaws in this code, so I need volunteers to play around with it, to spot errors and provide solutions. The sourcecode is at http://www.workshop-alex.org/Sources/TrayIcon/untTrayIconThread.html if you're interested.

There will be 100 points for a real major bug, 50 points for minor bugs and 20 points for nice suggestions. No more than 500 points will be given away, though, and those who spot bugs first will be the first to receive points. (But hey, feel free to continue making suggestions.) You yourself should describe if it's a major bug, minor bug or just a suggestion and I will check if you're right.

Oh, I know this trayicon object doesn't have much functionality. This is on purpose, since it's supposed to be used for debugging purposes only for other application. I would enable the debug mode in some way and the trayicon would provide me a way to make dumps of the internal process whenever I want to dump them. Of course, it could be made into a much more interesting thing or even a component, but for me it's extremely important that it doesn't need too much additional overhead.

Usage of this thing in e.g. a console application:

program Project1;

{$APPTYPE CONSOLE}

uses
  SysUtils,
  untTrayIconThread in 'untTrayIconThread.pas',
  untExtraInterfaces in 'untExtraInterfaces.pas';

type
  TDummy = class
    procedure SayHello;
    procedure SayBye;
  end;

var
  TrayIcon: ITrayIcon;
  Dummy: TDummy;

procedure TDummy.SayBye;
begin
  WriteLn('Bye');
end;

procedure TDummy.SayHello;
begin
  WriteLn('Hello');
end;

begin
  Dummy := TDummy.Create;
  TrayIcon := NewTrayIcon( 'TestAppA', 'Just a simple test.' );
  TrayIcon.AddTrayAction( 'Hello', Dummy.SayHello );
  TrayIcon.AddTrayAction( 'Bye', Dummy.SayBye );
  TrayIcon.Suspended := False;
  WriteLn( 'Press <Enter> to quit. ' );
  ReadLn;
  TrayIcon := nil;
  Dummy.Free;
end.

Feel free to give any comments about it that you like.
And if you feel this code has been useful for you, don't hesitate to provide positive feedback in my member profile. ;-)
0
Comment
Question by:Wim ten Brink
  • 8
  • 5
  • 3
  • +2
19 Comments
 
LVL 27

Assisted Solution

by:kretzschmar
kretzschmar earned 80 total points
ID: 12695528
looks good, alex, but by my lack of time, i can't help,
therefore following this thread
0
 
LVL 12

Assisted Solution

by:Lee_Nover
Lee_Nover earned 80 total points
ID: 12695572
cool idea about the links for units in the uses clause .. can I use that in my pas2html.dll ? :)
I'll check the TI out ..
0
 
LVL 17

Author Comment

by:Wim ten Brink
ID: 12695731
@Meikl, No problem. I'm not in a hurry. I haven't noticed anything wrong with it yet but just doesn't have enough time to text this thoroughly especially since I will only be using this code for debugging purposes of some other projects. (The trayicon allows me to dump debug information while the application just keeps running.)

@Lee, feel free to use this code for anything you like. I used GExperts to generate the HTML output, btw. Just edited the page to add a link to the second unit that it needs.

I don't expect any real nasty issues with this code and think it should run anywhere. If I had more time available I would remove the SysUtils unit from the code too. But am using that unit for some exception handling... The "TrayAction" method executes some arbitrary code defined elsewhere and theoretically this code could raise an exception. If I don't catch it then it crashes the thread. And if I do catch it, I should display it since there's nothing else that will be handling this error. Not using SysUtils would make the code a bit more unstable.
0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
LVL 5

Expert Comment

by:Hypoviax
ID: 12700388
Hmmm....

What's this code meant to do? Because when i run your sample app (having downloaded the needed units etc) nothing really happens. All i get is the <Press Enter to Exit> line.

Regards,

Hypoviax
0
 
LVL 17

Author Comment

by:Wim ten Brink
ID: 12705079
@Hypoviax, you should see a trayicon next to all other trayicons. However, since a console application doesn't have an icon, it's actually a simple gray block. You should add an icon to the console application by modifying the resource file. :-)
The trayicon is there, however. If you hold your mouse next to the right-most trayicon on the gray area, you probably would see a hint window pop up. If you click on it, a menuoption would have to be visible, showing two options. In the meantime, the console itself is just waiting for you to terminate the application again by pressing <Enter> which would kill the trayicon again.
If you would create a GUI application, the trayicon should be visible.
0
 
LVL 5

Assisted Solution

by:Hypoviax
Hypoviax earned 160 total points
ID: 12710322
It works well GUI. No problemo.

Comparitively this may interest you. It's written for GUI apps but it may be useful comparing to what you have:

uses ..., ShellApi;

type
  TForm1 = class(TForm)
    PopupMenu1: TPopupMenu; // popup menu for the tray with two options: show and exit
    mnuTrayShow: TMenuItem;
    mnuTrayExit: TMenuItem;
     procedure ToTray;
     procedure FormCreate(Sender: TObject);
    procedure FormActivate(Sender: TObject);
    procedure FormClose(Sender: TObject; var Action: TCloseAction);
    procedure mnuTrayShowClick(Sender: TObject);
    procedure mnuTrayExitClick(Sender: TObject);
  private
    { Private declarations }
  protected
    procedure WndProc(var Msg : TMessage); override;
  public
    { Public declarations }
    IconNotifyData : TNotifyIconData;
  end;

var
  Form1: TForm1;

implementation

{$R *.DFM}

uses unit2, Tools;

procedure TForm1.ToTray;
begin
  Form1.Hide;
end;

procedure TForm1.WndProc(var Msg : TMessage);
var
   p : TPoint;
begin
  case Msg.Msg of
    WM_USER+1:
    case Msg.lParam of
      WM_RBUTTONUP: begin // show popup menu with two options: show and exit
        GetCursorPos(p);
        PopupMenu1.Popup(p.x, p.y);
      end;
      WM_LBUTTONUP: begin
         mnuTrayShowClick(Self);
      end;
    end;
  end;
  if (Msg.Msg = WM_SYSCOMMAND) and (Msg.wParam = SC_MINIMIZE) then begin
    ToTray;
    Msg.wParam := 0;
    Msg.lParam :=0;
  end;
  inherited;
end;

procedure TForm1.FormCreate(Sender: TObject);
begin
  with IconNotifyData do begin
    hIcon :=Application.Icon.Handle;
    uCallbackMessage := WM_USER + 1;
    cbSize := sizeof(IconNotifyData);
    Wnd := Handle;
    uID := 100;
    uFlags := NIF_MESSAGE + NIF_ICON + NIF_TIP;
  end;
  StrPCopy(IconNotifyData.szTip, 'CD FTP');
  Shell_NotifyIcon(NIM_ADD, @IconNotifyData);
end;

procedure TForm1.FormActivate(Sender: TObject);
begin
  ShowWindow(Application.Handle, SW_HIDE);
  SetWindowLong(Application.Handle, GWL_EXSTYLE, WS_EX_TOOLWINDOW);
end;

procedure TForm1.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  Shell_NotifyIcon(NIM_DELETE, @IconNotifyData);
  Application.ProcessMessages;
end;

procedure TForm1.mnuTrayShowClick(Sender: TObject);
begin
  Form1.Show;
  Application.BringToFront;
end;

procedure TForm1.mnuTrayExitClick(Sender: TObject);
begin
  Close;
end;

end.

Regards,

Hypoviax

0
 
LVL 17

Author Comment

by:Wim ten Brink
ID: 12714415
Interesting, yes. But I wrote that other code myself to be part of some project I'm working on. Took me 4 hour to write and test it but I'm just not sure if it's really bugfree, which is why I posted this question. ;-)
I am quite familiar with trayicons, though. It's just that I want to have a non-intruisive way to use a trayicon for debugging purposes, which above unit archieves since it uses a separate thread and messageloop for each trayicon it generates. Your solution just hooks into the messageloop of an existing form. My trayicon could technically be used in a DLL or a console application and probably in a few other project types too.

My fear is that it just seems to be to easy to be bugfree. See, This unit combines several advanced Windows techniques. Multi-threading, trayicons, messageloops and critical sections. But it's less than 300 lines and it seems a bit too simple for me to be 100% confident about it. Still, I found no flaws with it. (except for invisible icons if the application has no icon.) Since I ran out of time to test this code, I just thought, let's share it and see if someone else can find any flaws in it. (Which is why I give it away too, free of charge.)

And hey, perhaps it is flawless and maybe someone else has a good use for it too. :-)

Oh, for a GUI application you could just do this:

uses ..., untTrayIconThread;

type
  TForm1 = class(TForm)
    procedure FormCreate(Sender: TObject);
    procedure FormClose(Sender: TObject; var Action: TCloseAction);
  private
    { Private declarations }
    TrayIcon: ITrayIcon;
  protected
    procedure SayHello;
    procedure SayGoodbye;
  public
    { Public declarations }
  end;

var
  Form1: TForm1;

implementation

{$R *.DFM}

procedure TForm1.FormCreate(Sender: TObject);
begin
  TrayIcon := NewTrayIcon( 'TestAppA', 'Just a simple test.' );
  TrayIcon.AddTrayAction( 'Hello', SayHello );
  TrayIcon.AddTrayAction( 'Bye', SayGoodbye );
  TrayIcon.AddTrayAction( 'Show', Show );
  TrayIcon.AddTrayAction( 'Hide', Hide );
  TrayIcon.Suspended := False;
end;

procedure TForm1.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  TrayIcon := nil;
end;

procedure TForm1.SayHello;
begin
  ShowMessage('Hello');
end;

procedure TForm1.SayGoodbye;
begin
  ShowMessage('Goodbye');
end;

end.

Be aware however that SayHello and SayGoodbye will NOT be executed in the main thread but in the thread related to the trayicon. Thus you can execute very long actions here, as long as you keep it thread-safe. Of course, the trayicon could also show and hide the application. You don't even need to add special functions in those cases since the methods Show and Hide are already of the correct type. Of course, I could overload the AddTrayAction method to accept some other kinds of callback functions too. (In which case I will need to remember the callback function type too and use the correct callback technique. But Hey, I don't have time for this.)
I did discover one flaw in a GUI environment, though. After:
  TrayIcon.AddTrayAction( 'Close', Close );
I get a nice Close action in my trayicon but since the Close is called from a different thread than the thread where the form was created, this will result in a nasty exception. Or actually 2:
> Exception EExternalException in module ntdll.dll at 0001FFE4.
> External exception C0000008.
and
> Exception EWin32Error in module SMSMessenger.exe at 0000A76C.
> Win32 Error.  Code: 5.
> Access is denied.
I've seen similar exceptions in other applications. Now I know why I might get them in my multi-threaded apps.
0
 
LVL 17

Expert Comment

by:geobul
ID: 12718012
Hi Workshop_Alex,

Looks very good indeed. Somehow I've missed this question and saw your last one first.

Just looking at the source code without testing anything and going deeper in understanding it well, there is something that perhaps could become a problem (it is very unlikely to happen or even impossible but anyway): You use CriticalSection.TryEnter (5 attempts) and don't handle situations if it fails. TTrayIcon.AddTrayAction is a good example: If TryEnter fails no action will be added and there is no negative feedback pointing that.

If the above is nonsense, please ignore my comment.

Regards, Geo
0
 
LVL 17

Author Comment

by:Wim ten Brink
ID: 12718779
Good comment, Geo. But it's actually done on purpose. :-)
TryEnter does allow you to specify the number of tries but by default it tries 5 times. If it cannot enter the critical section by that time, then I have to assume something is a bit too busy at that moment. The problem however is that the TrayIcon has it's own thread and raising exceptions within it will crash and terminate the thread if unhandled.
I've looked at the code after the TryEnter calls, btw. TryEnter will call a Sleep(0) if it can't enter the CS, so the other thread that keeps it locked gets time to continue processing. I don't think it's still locked after 5 tries.

However, in AddTrayAction I am in the main thread or whatever other thread is running at that moment so I could raise an exception at that location, which would have to be caught somewhere else.

I am evaluating now if I also need to check this critical section in the DeleteTrayAction method. I think I do have to add it there too, so you've pointed out a possible bug. (One which I never detected because I never deleted actions.)

One thing I fortunately noticed is that you can add actions from within an action. I don't execute the action from within the critical section, but just outside it.

Your comments make sense, though. One minor bugfix coming up soon. :-)
0
 
LVL 17

Accepted Solution

by:
geobul earned 1680 total points
ID: 12723832
My thoughts especially about AddTrayAction were that you could either use CriticalSection.Enter or make it a function returning a boolean result (the latter is not a standard for AddSomething methods, though), without raising an exception. I see what your point for using TryEnter is. On the other hand I have no information how EnterCriticalSection is implemented actually. Perhaps it also does something like you're alter.

My understanding of critical sections is that each provides access to a resource (or set of resources). If in that particular case the CS protects TrayActionList property then you have to use it everywhere you're accessing that property. Including DeleteTrayAction.

One another thing: in WindowProc function there is:
if TrayIcon.CriticalSection.TryEnter and ( Length( TrayIcon.TrayActionList ) > 0 ) then begin
Perhaps if you switch the conditions it'll be more correctly. The reason is that you may enter the CS but if there are no actions in the list and CS.Leave won't be executed.

Regards, Geo
0
 
LVL 17

Expert Comment

by:geobul
ID: 12723852
The last one is valid if {$B-} option is used. Otherwise splitting that IF into to would be better:

    if ( Length( TrayIcon.TrayActionList ) > 0 ) then begin
      if TrayIcon.CriticalSection.TryEnter then begin
        for Index := Low( TrayIcon.TrayActionList ) to High( TrayIcon.TrayActionList ) do begin
          AppendMenu( Menu, MFT_STRING, MENUITEM_ACTIONS + Index, PChar( TrayIcon.TrayActionList[ Index ].Name ) );
        end;
        TrayIcon.CriticalSection.Leave;
      end;
    end;
0
 
LVL 17

Author Comment

by:Wim ten Brink
ID: 12724641
Hmmm. I will have to enter the CS before I check the length of TrayActionList because an AddAction or DeleteAction could be changing the length at the same time. TrayActionList is the resource that needs to be protected here. I am not too worried about the items in this list since these are simple records that cannot be modified. They are either added or removed from the list. And when I use one in the WindowProc, I just make a copy of this record before using it, thus if the action is deleted from the list before I call it, I still have valid data.
I've modified AddTrayAction and DeleteTrayAction to just Enter the CS, without trying. But in the WindowProc I keep using TryEnter because this code is less critical in my opinion. If it fails after 5 attempts then too bad... I just don't want to block this thread too long. Also made sure to use a try-finally to make sure the Leave is always called.

Okay, so far Geo's comments resulted into the discovery of two minor bugs: the DeleteAction which was missing CS support and the {$B+} flaw. (An option which I NEVER use anyway.) And a good suggestion about the CS Enter/TryEnter code...
Still 380 points to divide left. ;-)
0
 
LVL 17

Expert Comment

by:geobul
ID: 12724828
Yep, I agree that entering the CS should be first. So, swapping the conditions in one if statement is not a valid option. The only option that remains is:

    if TrayIcon.CriticalSection.TryEnter then begin
      if ( Length( TrayIcon.TrayActionList ) > 0 ) then begin
        for Index := Low( TrayIcon.TrayActionList ) to High( TrayIcon.TrayActionList ) do begin
          AppendMenu( Menu, MFT_STRING, MENUITEM_ACTIONS + Index, PChar( TrayIcon.TrayActionList[ Index ].Name ) );
        end;
      end;
      TrayIcon.CriticalSection.Leave;
    end;
0
 
LVL 17

Author Comment

by:Wim ten Brink
ID: 12724924
Yep. But if you look at the source that I've linked to, you'll notice that I've already updated it to:

      if TrayIcon.CriticalSection.TryEnter then try
        if ( Length( TrayIcon.TrayActionList ) > 0 ) then begin
          for Index := Low( TrayIcon.TrayActionList ) to High( TrayIcon.TrayActionList ) do begin
            AppendMenu( Menu, MFT_STRING, MENUITEM_ACTIONS + Index, PChar( TrayIcon.TrayActionList[ Index ].Name ) );
          end;
        end;
      finally
        TrayIcon.CriticalSection.Leave;
      end;

The additional try-except should make it a bit more safe. There is a very slim chance that accessing TrayActionList results in an exception.

I don't like it though, that I cannot report any exceptions to the application but I need to keep this code multi-functional. I would hate to see an ISAPI dll that displays a messagebox because this code generated one... :-) Who is going to click it away? ;-)
0
 
LVL 17

Expert Comment

by:geobul
ID: 12725225
Try..finally is better because it's more safe. I have to refresh my browser from time to time ;-)
0
 
LVL 17

Author Comment

by:Wim ten Brink
ID: 12970910
Okay, closing up...

Bugs found:
kretzschmar: Thanks for the comment. :-)
Lee_Nover: You should award me points for giving you such an interesting suggestion for your pas2html.dll. ;-) But okay, this one is free.
Hypoviax: Do you know now what it's supposed to do? ;-) Oh, well... You're right. I need some example code for this unit sooner or later... And some help system, perhaps.
geobul: Yep, it looks good. :-) The part about the CriticalSection.TryEnter is a good suggestion, though. One minor bug...
geobul: Good comment about the CS that in some rare occasions might be entered but never leaves. Second minor bug.

Okay, Geo seems to be the only one who found some problems. Hypoviax provided some interesting piece of code that might be useful for some. All others, thanks for participating.

Dividing:
geobul: 420 points for fuinding the minor bugs
Hypoviax: 40 points for participating and the code snippet.
kretzschmar: 20 points for participating. :-)
Lee_Nover: 20 points for participating. :-)

I now know that 4 experts have at least looked at it and found it okay. Very reassuring. :-)
0
 
LVL 12

Expert Comment

by:Lee_Nover
ID: 12971010
:D tnx
0
 
LVL 17

Author Comment

by:Wim ten Brink
ID: 12972184
You're welcome... :)
0
 
LVL 5

Expert Comment

by:Hypoviax
ID: 12976556
Thanks,

You've become a bit generous of late.

Number 2 from the Wizard

Regards,

Hypoviax
0

Featured Post

Become an Android App Developer

Ready to kick start your career in 2018? Learn how to build an Android app in January’s Course of the Month and open the door to new opportunities.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

A lot of questions regard threads in Delphi.   One of the more specific questions is how to show progress of the thread.   Updating a progressbar from inside a thread is a mistake. A solution to this would be to send a synchronized message to the…
Objective: - This article will help user in how to convert their numeric value become words. How to use 1. You can copy this code in your Unit as function 2. than you can perform your function by type this code The Code   (CODE) The Im…
Loops Section Overview
Enter Foreign and Special Characters Enter characters you can't find on a keyboard using its ASCII code ... and learn how to make a handy reference for yourself using Excel ~ Use these codes in any Windows application! ... whether it is a Micr…
Suggested Courses

580 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question