I've developed a "single/multiple instances" feature for the Lazarus IDE. For this feature I need an IPC. First I tried to use simpleipc but I have struggled on bugs and missing functionality.

Particularly what I found:
1.) BUG: You can register multiple servers on Windows with the same name (with StartServer procedure).
2.) Small issue: simpleipc uses exceptions if something fails (e.g. in StartServer procedure). I would prefer using a function with boolean result (true = OK, false = fail).
3.) MISSING: Multiple clients (from different processes) talk to one server. 4.) MISSING: Client is able to receive a response on a request. 5.) MISSING: (Optionally, not default) client sends a request to a server that isn't running. The server is able to handle these requests when it is started.

So I developed "advancedipc.pas" that does what I need. It can do everything that is in simpleirc but a little bit differently (I changed the exception behavior and also method names). The question is now what to do?
1.) Include advancedipc.pas into FCL as a standalone unit. 2.) Make advancedipc.pas backwards compatible to simpleipc.pas and use the new code in simpleipc.pas.

I attached the advancedipc.pas file.
The discussion about "multiple instances" is here: http://bugs.freepascal.org/view.php?id=8051

Ondrej

Well, both 1 and 2 are the way to go if you want it included in FPC :-)

I see no point in including duplicate functionality, it means duplicate maintenance. You can keep it as separate units, just make sure simpleipc runs on top of advancedipc.

As for no semaphores on linux: of course there are semaphores, they exist since many many years, they are part of POSIX: "man sem_overview".

Michael.

.
.

One additional remark - it would be useful to have a test/example using advancedipc directly, so that users can base their code on it and platform maintainers can test easily and reliably whether the code works correctly on their platform (e.g. me on OS/2 ;-) ).

Tomas

Thanks a lot Michael!

I'll prepare it, write (unit) tests and some demo and then send them over! Thanks also for the semaphore information. I couldn't find anything on the internet, therefore I thought Lazarus doesn't support them.

Are there some high level crossplatform semaphore functions in the FCL? The cthreads/cIntSemaphoreOpen etc. seem to be available on unix only. What about OS/2?

What about Windows? It looks like semaphores are supported only since XP:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms682438(v=vs.85).aspx . Is this a problem for the FCL? (Basically, semaphores are only needed on unix due to the file locking. But I am still interested.)

If I'll have more questions, I'll write them here.

Ondrej

Careful, these are thread semaphores, I think.
I am talking about cross-process semaphores. They are in the baseunix/unix units.

I suppose OS/2 has semaphores, but Tomas will need to confirm. But if OS/2 currently does not support simpleIPC, then the point is moot anyway.

Officially, FPC no longer supports Windows 95. The compiler and most parts of the RTL will function on Windows 95, so it will do some basic stuff.

For the FCL, I really do not see this as a problem. If you need advanced IPC such as this, you will no longer be using Windows 95. We must be practical in such matters.

Michael.

Do I understand correctly that you are talking about "real" inter-Process communication and not inter-Tread-Communication within the same project ? (IMHO, this would require completely different paradigms.)

I understand that IPC is done with non-Lazarus projects in mind. Hence it is done to be used "straight forward" independent of the Lazarus Event Queue. If doing something on top of SimpleIPC it might be viable to consider Event Queue integration for Lazarus users, as well (but If you don't consider Lazarus this might be a completely unrelated issue).

-Michael

IPC = Inter PROCESS Communication.

Michael.

Michael Van Canneyt wrote

IPC = Inter PROCESS Communication.

I know that you know this, but Ondrej is talking about cthreads in one of his message, so I tried to politely set a trigger.

-Michael

This is because Michael Van Canneyt pointed me to "man sem_overview". The first Google search pointed me to
http://linux.die.net/man/7/sem_overview where I found the "Named semaphores" with functions sem_open, sem_close etc. with the description that they can be used across processes. A quick search in FPC sources pointed me again to the cthreads unit. I haven't done any Linux development with semaphores, so sorry if I am completely wrong here. I will also welcome if you correct me.

BUT semaphores are not directly needed by the IPC. simpleipc doesn't use them either.

It would be nice to have solved the issue with double file locks on unix, but this is not the core problem.

Ondrej

All semaphore use has been removed from the cthreads unit in the mean time.

They also should not be used on OS X anymore, because Apple forbids their use in applications submitted to their appstore (I guess because it is difficult for system frameworks to automatically determine whether they are only used in safe way; i.e., in a way that doesn't make the codesigned host application into a proxy for any other application to carry out operations on their behalf).

Jonas

With my paranoid mind I of course assumed you were hijacking the thread...

Michael.

Michael Van Canneyt wrote

With my paranoid mind I of course assumed you were hijacking the thread...

Of course I was eagerly waiting on Ondrej to confirm that he was looking for thread support :-) :-) :-)

-Michael

Michael Van Canneyt wrote

On Tue, 15 Sep 2015, Ondrej Pokorny wrote:

.
.

OS/2 certainly has semaphores (three types of them, in fact ;-) - event semaphores, mutex semaphores and muxwait semaphores).

But if OS/2 currently does not support simpleIPC, then the point is moot anyway.

See packages/fcl-process/src/os2/simpleipc.inc. Moreover, the code posted by Ondrej does not contain anything that should not compile on OS/2 at the first sight, that's why I wanted to have a test allowing to check whether it really works. The fact that sending messages across processes would be usually implemented differently if using OS/2 API features directly may not be so important.

Tomas

Excellent !

Well, the implementation looks as if it will work on all platforms.

Michael.

and there's also file based semaphores still used by a huge number of programs to signal needed actions between disparate programs that may be pressed into operation together ;)

A quick check of the code I posted before you tried to politely set a trigger would have spared you the trouble :-) :-) :-)

---
Michael Van Canneyt & Tomas Hajny & everybody who wants to test:

I prepared a fully compatible AdvancedIPC unit. See the attachment. There is a unit mysimpleipc.pp that should replace the simpleipc.pp in FPC sources. I renamed it in order to prevent file name problems and to be able to check the compatibility without recompiling FPC sources (basically just use "mysimpleipc" in the uses instead of "simpleipc").

Description of projects:
dbugsrv.lpi, ipcclient.lpi, ipcserver.lpi: demos from FPC sources just to check backwards compatibility.
TestIPC_Client.lpi + TestIPC_Server.lpi: server/client test of new AdvancedIPC features.

I haven't implemented any global semaphores etc to work around the file locking problem on unix. They were not present in simpleipc, they are not present in the TFileStream implementation and as I read the discussion about the problem, I'd rather do it without them and keep the limitation. Maybe I should write about this limitation in the header.

I also modified the internal message header. That means that both client and server must be compiled with the new units. But this is a must anyway because the whole back-end code changed (e.g. on Windows from messages to temporary files). I have a "FileLock" field in the header as well that serves as an extra file lock flag. This is not needed under Windows because file locks work there but my tests on Linux showed that file locks work only across different processes. The same process seems to be able to access a locked file from 2 FileCreate calls (please correct me if I am wrong). So this is just a bullet-proof check. If you think it is not needed, I remove it.

If you have questions about the code, feel free to ask or if you found some issues/bugs, please tell me about them as well.

Ondrej

Ondrej Pokorny wrote

Michael Van Canneyt & Tomas Hajny & everybody who wants to test: I prepared a fully compatible AdvancedIPC unit. See the attachment. ...

Michael and Tomas, I am planning to copy AdvancedIPC temporarily to Lazarus sources.
If you are planning changes for it, I can wait.
The temporary copy does not need to be identical but it should be API compatible.
With it we can finally implement the single IDE instance feature without depending on the latest FPC libs.
http://bugs.freepascal.org/view.php?id=8051

Regards,
Juha

Unfortunately, I didn't have time for testing it yet, sorry. :-( If you can wait until end of this week, I'll make sure to do so during the weekend at the latest.

Tomas

I added it to fpc/packages/fcl-base. It compiles OK, there are no dangerous dependencies except sysutils and classes.

Juha, if you want to implement the single IDE instance, then I would like to ask you to implement this in custapp.pp. This is useful functionality for all kinds of applications, and is since very long on my todo list.

I think the functionality can be split out in 2 parts:
- Make sure a single instance is running.
- Sending messages to running instance.
The first can be implemented in custapp, I am not sure about the second. At first sight, I think that this should be possible.

Michael.

Michael Van Canneyt wrote

I added it to fpc/packages/fcl-base. It compiles OK, there are no dangerous dependencies except sysutils and classes.

Thanks!

I don't think it is needed (see comments further down).

I think the functionality can be split out in 2 parts: - Make sure a single instance is running.

The IPC does it by design - you start a server and you have the check, so you get it. IMO it's not needed to make another component above AdvancedIPC with exactly the same functionality but different name.

- Sending messages to running instance.

Again, IPC does it.

Actually, you can achieve "single instance" with AdvancedIPC with only a few lines of code (I think the simplest code would be under 10 lines). Every application can have different needs about the "single" instance so a customization is needed anyway (e.g. passing over other command line parameters etc.)

Juha, I changed the advancedipc.pas unit (this one from the mailing list is newer than from the bug report). I can prepare a new patch for the bug report.

Ondrej

I think it is very much needed.

I am not proposing to make a new class.
I want a property "SingeInstance" in CustApp which activates this automatically.

> - Sending messages to running instance. Again, IPC does it.

I know that, I simply want it integrated in CustApp.

By doing this, 'Single instance' can be a simple flag in the project options.

You may not think this is important, but for beginners, this can be a tremendous aid. The easier we make it, the better. Lazarus is a RAD environment, after all.

Michael.

OK, I see your point. Yes, I think it is possible and even not that much work to do.

I programmed "multiple instances" handling into Lazarus IDE. It's the same like e.g. Adobe Photoshop or VLC player work - if you start the IDE without files in cmd parameters, the IDE is started. If you start IDE with files, the files are opened in already running instance. You can enable a truly "single instance" (always only single instance allowed) with an environment setting option.

IMO, we should keep the code in CustApp simple and don't do this advanced stuff. Only single/multiple instances option. If single instance is selected, allow to talk with the one single instance through IPC. If there are multiple instances, you cannot talk to other instances. This should be doable.

What do you think?

Ondrej

Exactly what I had in mind, simple yet functional :-)

Michael.

OK, I'll prepare this one as well. But the Lazarus IDE won't take advantage of CustApp (it will use AdvancedIPC directly with the custom advanced possibilities I already programmed).

Ondrej

Michael Van Canneyt wrote

I am not proposing to make a new class. I want a property "SingeInstance" in CustApp which activates this automatically.

I had the very same idea when this was discussed a long time ago in Lazarus list.
Martin Friebe noted that every application would then depend on this single-instance IPC code.
It was a valid point. There should not be such a dependency.

The right way is to use a drag-and-drop LCL component like the one from Luiz Américo.
The question is should we replace his component with a better one based on Ondrej's AdvancedIPC.
I think we should because SimpleIPC apparently has limitations.

Michael, an LCL component is as beginner friendly as your SingeInstance property would be.

Regards,
Juha

Ondrej Pokorny wrote

Juha, I changed the advancedipc.pas unit (this one from the mailing list is newer than from the bug report). I can prepare a new patch for the bug report.

No need for a new patch if it did not change otherwise. I only would like to move the AdvancedIPC unit to somewhere else from ide directory.
I can ask opinions at Lazarus developer list.

Juha

Michael Van Canneyt wrote

I added it to fpc/packages/fcl-base. It compiles OK, there are no dangerous dependencies except sysutils and classes.

Michael, you added the wrong (old) unit from the bug report, not the latest simpleipc-compatible one from the mailing list. Please apply the patch from the attachment!

Thanks
Ondrej

For simpleIPC, yes. But the new implementation has no dependencies except sysutils, classes. So the argument becomes void.

That's why I was glad with Ondrej's implementation.

The right way is to use a drag-and-drop LCL component like the one from Luiz Américo.

That is IMHO a completely wrong implementation for 2 reasons:

1. Using a LCL component means that you must instantiate a form/datamodule first. By that time, a database connection may already have been made (just to name something), which is exactly what you want to avoid.

The check for single instance should be done before the first form is created.

2. Then it is only available in visually designed applications. Services, website servers and custom console apps are non-visual. I am specificially thinking FastCGI processes.

All these have in common that they descend from TCustomApplication.

That is why I think TCustomApplication is the only good location;
So if Ondrej does not do it, I will add it myself anyway.

Lazarus is free not to use it, obviously.

Michael.

Strange. I took the one from the mail ? Maybe some name mixup :(

My apologies. Applied the patch, rev. 31890. Thanks !

Michael.

Michael Van Canneyt wrote

I added it to fpc/packages/fcl-base. It compiles OK, there are no dangerous dependencies except sysutils and classes.

Michael, you added the wrong (old) unit from the bug report, not the latest simpleipc-compatible one from the mailing list. Please apply the patch from the attachment!

Thanks
Ondrej

Sorry for the double email, I am using mobile internet now and it reported mail send error.
Thanks for committing the patch, maybe it was my mistake, no problem anyway. It's OK now!

Ondrej

The API may have changed a little bit. I'll recheck it. Ondrej

Michael Van Canneyt wrote

For simpleIPC, yes. But the new implementation has no dependencies except sysutils, classes. So the argument becomes void.

It adds extra code to every application. Maybe not too much though.

... That is why I think TCustomApplication is the only good location; So if Ondrej does not do it, I will add it myself anyway.

Ok, I see your point. I am not arguing against it.

Lazarus is free not to use it, obviously.

TApplication inherits from TCustomApplication, thus it will be included in every LCL application.

Juha

Actually, you could create the LCL component manually in the LPR and use it non-visually before any code is run (except initialization sections). The visual component could be only for the hobbyists.

So for me both CustApp or LCL component are OK. You decide :)

Ondrej

Both are OK. Use of one does not exclude use of the other.

I was going to add it in CustApp anyway as soon as I had looked at your implementation.

Michael.

2015-09-29 10:35 GMT-03:00 Michael Van Canneyt :

It can be used without the component. The component is just a convenience. See the examples

It can be done

See above

All these have in common that they descend from TCustomApplication.

And console apps without TCustomApplication?

Luiz

2015-09-29 11:50 GMT-03:00 Luiz Americo Pereira Camara <

https://github.com/blikblum/luipack/blob/master/uniqueinstance/testraw/project1.lpr

Luiz

I didn't say it could not be done with that component. But IMHO you should have gone the extra mile and implement it in TCustomApplication.

Ondrej's class has the advantage that it introduces no extra dependencies. It builds on classes and sysutils.

> All these have in common that they descend from TCustomApplication. > > And console apps without TCustomApplication?

They can use advancedipc directly if they want. I rarely make those any more for production,
- Because the command-line handling of TCustomApplication is convenient.
- Most often you need a class anyway for event handling or to use as owner. We must assume something.
The minimum shared code is in TCustomApplication, it is the most logical place.

Michael.

unit uniqueinstance;
interface
uses
   Forms, Classes, SysUtils, simpleipc, ExtCtrls;

Forms, ExtCtrls ??

This disqualifies your component fully for anything but GUI apps.

Michael.

Michael Van Canneyt wrote

Both are OK. Use of one does not exclude use of the other. I was going to add it in CustApp anyway as soon as I had looked at your implementation.

I started working on the CustApp variant. It's even simpler than the LCL component because you can handle the check in DoRun() and so you don't need any timer or watch thread, if I am correct.

Ondrej

Unit uniqueinstanceraw

Please look carefully

Luiz

Luiz Americo Pereira Camara wrote

Unit uniqueinstanceraw Please look carefully

My sincere apologies,

I was too fast and indeed opened the wrong unit. uniqueinstance, and uniqueinstancebase, not uniqueinstanceraw :(

So, it could be used as well, if we can sort out the dependency on simpleipc.

Michael.

That was what I thought too.

What I failed to see were provisions for per-user and global instances, maybe I missed them. If not, these can be addressed later on.

Michael.

2015-09-29 16:12 GMT-03:00 Michael Van Canneyt :

No problem.

To be clear, i don't plead to uniqueinstance be used by fpc or Lazarus at all.

This is the message i sent to Lazarus dev list when discussing the possibility of using in Lazarus IDE:

"
See
https://github.com/blikblum/luipack/blob/master/uniqueinstance/uniqueinstanceraw.pas and
https://github.com/blikblum/luipack/blob/master/uniqueinstance/uniqueinstancebase.pas

This all the code that is needed to implement unique instance feature on top of simpleipc.
Such simple logic could even be copied to IDE instead of using the package itself
"

On the other side, i still think that should be kept out of TCustomApplication. While the simple requirement of checking another instance is easy to implement into TCustomApplication, others may want more features / control (See bug report for examples). Those that need it would duplicate part of the code.

Maybe create fpUniqueInstance on top of Ondrej work, making extensible and plugabble?

Luiz

Luiz Americo Pereira Camara wrote

On the other side, i still think that should be kept out of TCustomApplication.

Well, I have already explained why I want it exactly there :)

While the simple requirement of checking another instance is easy to implement into TCustomApplication, others may want more features / control (See bug report for examples). Those that need it would duplicate part of the code.

Yes and no, it depends on how you expose the feature. When I said I want a single property, I meant of course that it can be enabled in it's most basic form with just property. That does not exclude that more properties can be exposed to offer more functionality.

The big advantage of Ondrej's functionality is that it will work on all platforms. SimpleIPC will not work on all platforms.

Maybe create fpUniqueInstance on top of Ondrej work, making extensible and plugabble?

Yes. I had thought about this too, we are of the same idea :-)

Let's first see how Ondrej does it.

Michael.

Michael Van Canneyt wrote

Let's first see how Ondrej does it.

See the patch attached. A simple demo program is attached as well (SITest.lpr).

advancedipc.pp: refactoring MsgID -> RequestID and some smaller issues solved. API hasn't changed.
singleinstance.pp: class implementing single instance. custapp.pp: added singleinstance property. Set Enabled to True (before CustomApplication.Initialize) and it does everything for you. You can also set more properties like ID and Global.
You can talk to the server single instance after Application.Initialize if you want to (see SITest.lpr).

+ I am now travelling so I have only MS Windows to check. There is a workaround for UNIX to solve the filelock problem. The problem is that if you execute more files from the explorer, the instances are started at the same time so there could be problems with creating the file lock (double file locks can occur). The clue is that you wait for other instances to start (timeout 100ms) and then use the highest instance as server. If there are some pending wait-requests (if the program collapsed or something like that) there is a random sleep to decide what instance should be the server. This approach is very bullet-proof, as my tests showed (the random sleep is not needed in normal situations).

+ A small issue is that checking for new messages in CustomApplication.DoRun too often uses a lot of CPU power. The check should be made only about every 50ms (then the CPU needed is at ~0%). I don't know how to do it the best way, because eg. using GetTickCount needs also a lot of CPU. I don't know how it behaves with TApplication (maybe there is no such problem).

+ I had a problem with random numbers (I checked windows implementation only). If randomize is executed from 2 different processes at exactly same moment, it returns the same randseed (GetTickCount). This is a big problem with processors with multithreading. In this case you can have exactly same random sequences for 2 different threads/processes. I suggest to include GetCurrentThreadID into randomize somehow.

Something like:
   procedure randomize;
   begin
     randseed:=Integer(Int64(GetTickCount)+GetCurrentThreadID);
   end;

Addition is maybe not the best operation, but you should get the idea. I solved this problem by adding GetCurrentThreadID to the random result, so advancedipc is not affected by this problem any more.

+ Juha, I will upload a new patch for Lazarus IDEInstances. Probably nothing changed, but just to be sure.

Ondrej

On 21 Sep 15, at 14:03, Ondrej Pokorny wrote:
.
.

.
.

Thanks, I tested it under OS/2 now. Both the "simple" and "advanced" test programs work, well done! I noticed two issues when running the "advanced" tests, though. First, the server increases the CPU considerably. There seems to be a loop permanently searching for a file FindFirst without any sleep in between. Maybe related to PeekRequest calls? Second, I noticed that unlike the "simple" test, the "advanced" leaves a temporary file 'hello-NNNN-t' in the temporary directory after finishing.

Apart from that, I replaced the hard-coded '*' with a reference to AllFilesMask constant defined in unit System, because some of FPC targets need a different mask when searching for all files (that is not the case of OS/2, but e.g. GO32v2 would need that if not running on a LFN enabled system) - committed in svn trunk.

Tomas

First - Yes that's true, I didn't add any sleep into the code for testing purposes.
Second - I'll check that!

Thanks! I found some smaller bugs that I send over to the temporary copy in Lazarus trunk. I'll sync both files and send patches back.

Ondrej

Attached patched for FPC and Lazarus to sync advancedipc.pp. The current version in FPC trunk is out-dated (the Lazarus trunk version just misses the AllFilesMask constant).

I could not reproduce the "temporary file 'hello-NNNN-t' was not deleted" problem. Actually all files should be deleted in StopServer(True) or Free -> DeletePendingRequests.

What demo did you observe the problem in? TestIPC_Server/TestIPC_Client?

Ondrej

The solution is simple and is only a problem of the demo application (not advancedipc.pp code). Just add an "ELSE SLEEP(XYZ)" block into the server while-loop.

Ondrej

Ondrej Pokorny wrote

On 03.10.2015 19:21, Tomas Hajny wrote:

.
.

Attached patched for FPC and Lazarus to sync advancedipc.pp. The current version in FPC trunk is out-dated (the Lazarus trunk version just misses the AllFilesMask constant).

Thanks, I applied a modified version of your patch to FPC SVN; since we put the file to FPC trunk, the modifications related to support of FPC 2.6.4 would be superfluous there and I skipped them.

Yes. I cannot reproduce it either, though. I guess that it must have resulted from me killing the server at some point (due to the high CPU load), i.e. apparently a false alarm - sorry. :-(

Tomas

Yes, indeed, that fixed it, thanks. I've added the test / example to FPC SVN under the directory examples.

Tomas

Michael Van Canneyt wrote

Juha, if you want to implement the single IDE instance, then I would like to ask you to implement this in custapp.pp. This is useful functionality for all kinds of applications, and is since very long on my todo list.

Hello Michael,

you asked for implementing "single instance" in custapp.pp. I did it and sent a patch on Sept 28th. Did you have the time to check it and find eventual issues?

I am resending the patch against current trunk and a testing application.

Ondrej

Well, I had an uncommitted patch for custapp.pp at the time (better non-options and multiple-options support), which made applying and testing quite difficult. So I waited, but forgot about it.

I checked the patch and applied it, so people can try it.

I have several remarks:

a) Your TBaseSingleInstance class contains too many methods. It assumes you are using advancedipc.
I suggest refactoring such a way that advancedipc is in the implementation section of the class.

or introduce TAbstractSingleInstance as a parent of TBaseSingleInstance with the bare minimum of methods/properties.

b) There is no way to have TCustomApplication create a different singleinstance class, for 2 reasons:
1. Your property is declared as TCustomSingleInstance. It should be TBaseSingleInstance (or TAbstractSingleInstance) That means that the 'enabled' property should be in TBaseSingleInstance or TAbstractSingleInstance.
2. You create the instance as TCustomSingleInstance.Create in the constructor.

      It should be a function CreateSingleinstance : TCustomSingleInstance;

Can you have a look at these remarks ? If something is not clear, let me know.

Michael.

Great!

Your remarks are reasonable. I'll rewrite the code so that it supports modifications!

Ondrej

Michael Van Canneyt wrote

I have several remarks: a) Your TBaseSingleInstance class contains too many methods. It assumes you are using advancedipc.

I refactored TBaseSingleInstance so that it doesn't depend on advancedipc.

I suggest refactoring such a way that advancedipc is in the implementation section of the class.

This is unfortunately not possible now because there are no interfaces (nor abstract classes) for TIPCServer and TIPCClient. IMO this is not a problem, if you don't want singleinstance.pp to be dependent on advancedipc.pp at all, TAdvancedSingleInstance can be easily moved into another unit - e.g. "advancedsingleinstance.pp". CustApp.pp will need to use "advancedsingleinstance.pp" in implementation section then.

If you want to have abstract classes for TIPCServer and TIPCClient, advancedipc.pp would need a bigger refactoring. Adding interfaces for them seems to be a simpler and better solution for me. - But as I said before, IMO neither interfaces nor abstract classes are needed for now. TBaseSingleInstance already introduces the bare minimum of methods needed.

or introduce TAbstractSingleInstance as a parent of TBaseSingleInstance with the bare minimum of methods/properties.

TBaseSingleInstance is now such "TAbstractSingleInstance". You can rename it to TAbstractSingleInstance if you like.

The Enabled property doesn't belong into TBaseSingleInstance - it has a meaning only in TCustomApplication. I solved it by introducing SingleInstanceEnabled in TCustomApplication.

2. You create the instance as TCustomSingleInstance.Create in the constructor. It should be a function CreateSingleinstance : TCustomSingleInstance;

I solved it by introducing SingleInstanceClass. IMO it's better because SingleInstanceClass can be easily changed without the need to create a new TCustomApplication descendant and override a virtual function.

Feel free to comment on my changes.

BTW, there are some compiler hints/warnings in CustApp.pp. At least the one warning should be solved, IMO:

Compile Project, Target: sitest.exe: Success, Warnings: 1, Hints: 8
custapp.pp(175,6) Note: Local variable "l" is assigned but never used
custapp.pp(57,21) Hint: Parameter "EventType" not used
custapp.pp(57,51) Hint: Parameter "Msg" not used
custapp.pp(391,25) Hint: Local variable "B" does not seem to be initialized
custapp.pp(408,29) Warning: Local variable "I" does not seem to be
initialized
custapp.pp(408,27) Hint: Local variable "B" does not seem to be initialized
custapp.pp(453,30) Hint: Local variable "B" does not seem to be initialized
custapp.pp(502,31) Hint: Local variable "B" does not seem to be initialized
custapp.pp(524,8) Note: Local variable "SO" is assigned but never used

Ondrej