-TCx64 opens twice despite of allow 1 copy policy

[ghisler(Author)]

Unfortunately this cannot always be prevented: The function checks whether there is an open Total Commander main window. I cannot check for the presence of the process only, because then it would also be impossible to open separate search, compare, Lister, or sync dialogs.

[Ovg]

IMHO this issue is very rare and user should special try to rise it so don't care about this

[Dark-Star]

Why not simply create a named Mutex on TC start? This is what I do to make sure my programs start only once. You can use the result of the CreateMutex() function to check if it was successful in creating the mutex (then TC should start) or if the mutex already exists (then it should just quit)

[MVV]

Dark-Star,
This way may be good only for 1 instance, but we need same code for N instances where N is set by user.

[Dark-Star]

MVV,
Since when does TC support something different from "only 1 instance" and "as many instances as you want"?

But even then there's always named semaphores which can do exactly that, so I don't see a problem?

The point is that mutexes/semaphores are perfectly atomic, so there's no way that you can "sneak" past another instance of TC, no matter how fast you are with starting them

[Horst.Epp]

MVV,
Since when does TC support something different from "only 1 instance" and "as many instances as you want"?

But even then there's always named semaphores which can do exactly that, so I don't see a problem?

The point is that mutexes/semaphores are perfectly atomic, so there's no way that you can "sneak" past another instance of TC, no matter how fast you are with starting them

What happens with the mutexe/semaphore if TC crashes.
Is it cleared automatically or must you logout to get rid of it before you can start TC again.

[Dark-Star]

It gets cleared automatically by windows when the process terminates (just like all open files get closed etc.)

[MarcinW]

I can confirm, that the operating system closes mutexes and semaphores automatically on process termination. MSDN also says that.

FindWindow will hang, if one of existing TC instances is hanging. I experienced this many times - when one TC instance is hanging due to some hardware/driver problem with USB drive, other TC instances cannot start.

Here is my proposal with mutexes. It:
- detects reliably if the instance is unique,
- detects reliably if the instance number ([2], [3], [...] in the caption) is free,
- works properly even with Windows 95,
- works properly also with Windows Server with multiple users logged on - each logon session has its own "uniqueness" and caption numeration.

Test.dpr:

Code: Select all

program Test;

uses
  UniqueInstance {This should be the first!},
  Windows, SysUtils;

var
  S : string;
begin
  if InstanceIsUnique then
    S:='First instance'
  else
    S:='Another instance';

  S:=S+' ['+IntToStr(InstanceNumber)+']';

  MessageBox(0,'Test',PChar(S),MB_ICONINFORMATION or MB_TOPMOST);
end.

UniqueInstance.pas:

Code: Select all

unit UniqueInstance;

interface

var
  InstanceIsUnique : Boolean;
  InstanceNumber : Integer;

implementation

uses
  Windows;

function IsInstanceUnique(const MutexName : string; CloseHandleIfNotUnique : Boolean) : Boolean;
var
  MutexHandle : THandle;
begin
  {"The system closes the handle automatically when the process terminates."}
  MutexHandle:=CreateMutex(nil,False,PChar(MutexName));
  if MutexHandle <> 0 then
  begin
    if GetLastError = ERROR_ALREADY_EXISTS then
    begin
      Result:=False;
      if CloseHandleIfNotUnique then
        CloseHandle(MutexHandle);
    end else
      Result:=True;
  end else
  begin
    {ERROR_ACCESS_DENIED means that mutex exists}
    if GetLastError = ERROR_ACCESS_DENIED then
    begin
      Result:=False;
      if not CloseHandleIfNotUnique then
        OpenMutex(SYNCHRONIZE,False,PChar(MutexName));
    end else
      Result:=True;
  end;
end;

function Check : Boolean;
const
  MutexName = 'Unique Program Name';
var
  NumberStr : string;
  I : Integer;
begin
  {IMPORTANT: this should be called only if NOT launched as a
   TC helper instance (Lister, Separate search, etc.), because
   IsInstanceUnique(..,False) leaves the mutex open}
  InstanceIsUnique:=IsInstanceUnique(MutexName,False);

  Result:=InstanceIsUnique;

  for I:=1 to High(I) do
  begin
    {wvsprintf, lpOut: "The maximum size of the buffer is 1024 bytes"}
    SetLength(NumberStr,1024);
    SetLength(NumberStr,wvsprintf(PChar(NumberStr),'%u',PChar(@I)));
    if IsInstanceUnique(MutexName+' '+NumberStr,True) then
    begin
      InstanceNumber:=I;
      Break;
    end;
  end;
end;

initialization
  {We use a separate function to execute the checking code, so dynamic
  local variables, like NumberStr, will be automatically cleared before
  the function returns. Thanks to this, the checking code will not leave
  any allocated memory, so this unit can be used (listed in the unit list
  of the main .dpr file) even before memory managers, like FastMM4}
  if not Check then
    {Halt(1);}
end.

Note: There is a separate unit - UniqueInstance.pas - created in order to execute the detection code. This unit should be placed as the first unit on the unit list in the main .dpr file. Thanks to that, this unit will be initialized at the very beginning of the program execution and its code will be executed before the process will be preempted by the operating system (which may lead to launching many application instances before creating any mutex). Some units may execute their initialization code for as long as 20 ms, which is a standard thread time-slice in consumer Windows (servers have 10 ms), so the main thread of the process may be preempted there - so it's important to be the first and execute the code before any time-consuming initialization operations.

It's also important to note that:
- UniqueInstance unit uses only the Windows unit, which has no initialization code (or very little code in recent Delphi versions); so Windows unit will be in fact initialized before executing the UniqueInstance initialization code, but initialization of the Windows unit will be very fast.
- UniqueInstance unit does not leave any memory allocated (like dynamic strings), so it can be listed in the main .dpr file even before alternative memory managers, like FastMM4.

Regards

[MVV]

Since when does TC support something different from "only 1 instance" and "as many instances as you want"?

It does it since I can't even remember which exactly version.

Onlyonce=0

1: Start only one instance of Total Commander per user (e.g. started via RunAs)
-1: Start only one instance of Total Commander on this computer (for all users)
x>0: Same as 1, but multiple allowed per user
x<0: Same as -1, but multiple allowed on that computer (all users together)

So, Onlyonce=5 will allow 5 instances per user.

But even then there's always named semaphores which can do exactly that, so I don't see a problem?

Hm, it seems that semaphore may be a good choice here.

The point is that mutexes/semaphores are perfectly atomic, so there's no way that you can "sneak" past another instance of TC, no matter how fast you are with starting them

How often do you start multiple TC instances simultaneously? Collision usually occurs when you start new instance when previous one was already started but haven't created a window yet. If you start instances one by one, they usually see existing ones correctly (except when some ones are elevated: non-elevated TC can't activate elevated one).

It gets cleared automatically by windows when the process terminates (just like all open files get closed etc.)

That's correct, OS always frees all resources acquired by a process on its termination.

FindWindow will hang, if one of existing TC instances is hanging.

Actually FindWindow[Ex] doesn't hang when processes are not responding, try this with suspended Notepads, it finds all windows with no delay:

Code: Select all

		HWND wnd=0, wndarr[256]; int i=0;
		while (wnd=FindWindowEx(0, wnd, L"Notepad", 0)) wndarr[i++]=wnd;

But SendMessage and other functions that interact with hung windows may hung, that's why SendMessageTimeout function exists.

Here is my proposal with mutexes. It:

It is a very bad idea to create N mutexes for N instances. We shouldn't waste system resources in such a way. Semaphore suggested by Dark-Star is a much better alternative since we need only one kernel object. However I can't imagine yet how to detect another user instances in case of semaphores. But stop, it seems that semaphores may be created in global or session namespaces (with prefixes "Global\" or "Local\") and it is enough for required detection (but it seems that we'll need two kernel objects, global and local ones).
Also please note, when TC doesn't start new instance, it activates existing one instead of just exiting.
Finally, special TC instances like Sync, Lister, Search should never check for existing instances or increase counters, they should simply start always. And, TC with /N parameter should only increase counter but shouldn't check for a limit. But it requires to parse command line in order to detect such cases so unit initialization code is not a good place for synchronizing.

[Dark-Star]

So, Onlyonce=5 will allow 5 instances per user.

Good to know... I don't usually fiddle with INI settings

How often do you start multiple TC instances simultaneously? Collision usually occurs when you start new instance when previous one was already started but haven't created a window yet.

I usually don't, but the whole point of this thread was what happens when you start instances faster than they can create their main windows, which was what I was referencing here.

It is a very bad idea to create N mutexes for N instances. We shouldn't waste system resources in such a way. Semaphore suggested by Dark-Star is a much better alternative since we need only one kernel object. However I can't imagine yet how to detect another user instances in case of semaphores. But stop, it seems that semaphores may be created in global or session namespaces (with prefixes "Global\" or "Local\") and it is enough for required detection (but it seems that we'll need two kernel objects, global and local ones).

Yes, you can create global and local kernel objects. However I doubt that it would be a huge problem, resource-wise, if TC created n mutexes, one for each instance. I have seen programs create dozens of named mutexes for each instance, and that wasn't a problem either. Remember, we're talking "a handful" here, not "thousands"

[MVV]

Good to know... I don't usually fiddle with INI settings

It is really useful to check HISTORY.txt for missed new features, also TOTALCMD.chm section 4.b (describes all INI options).

Remember, we're talking "a handful" here, not "thousands"

There are users that create more than 2000 usercommands, who knows, maybe there are ones who start 2000 instances.

Problem with multiple mutexes is that we can't enumerate them so we should try opening them in some kind of loop, but at which N should it be stopped (one can start 5 instances, then close 4th, but we can't stop after 3rd, there may be 5th and other)?

[MarcinW]

FindWindow will hang, if one of existing TC instances is hanging.

Actually FindWindow[Ex] doesn't hang when processes are not responding, try this with suspended Notepads, it finds all windows with no delay

This:

Code: Select all

FindWindow('Notepad',nil)

will not freeze, but this:

Code: Select all

FindWindow('Notepad','Test.txt - Notatnik')

may freeze, because FindWindow must check the window name, so in consequence WM_GETTEXT message must be sent to this hanging Window.

But I just tested this with Notepad as you suggested and I must admit, that FindWindow doesn't freeze. It is probably because Windows caches last window title and returns this cached title when the window is hanging. This caching has been probably introduced in Windows 2000.

It is a very bad idea to create N mutexes for N instances. We shouldn't waste system resources in such a way.

Don't exaggerate, I just launched Process Monitor and I could see, that TC uses about 20 kernel objects. In fact, every open file is a kernel object. Is one more mutex a big problem? Rather not. And how many instances of TC can we launch...

But stop, it seems that semaphores may be created in global or session namespaces (with prefixes "Global\" or "Local\") and it is enough for required detection (but it seems that we'll need two kernel objects, global and local ones).

I used the "Global" namespace for shared bitmaps, and I found the following information:

On Windows XP, there was no problem. We named our segments 'Global\Something' and all was well. The additional security in Vista (and assumedly Windows 7) appears to prevent this architecture from working. Normal users are not allowed to create (Win32 error 5) objects in the global namespace. The MSDN indicates that if the account has the "create global" privilege then all should be well, but this does not seem to be the case in practice.

Also please note, when TC doesn't start new instance, it activates existing one instead of just exiting.

Not really a problem - after the mutex handling, TC can finally activate that window - and this may be done in any place of the program, no need to do this at the very beginning.

Finally, special TC instances like Sync, Lister, Search should never check for existing instances or increase counters, they should simply start always. And, TC with /N parameter should only increase counter but shouldn't check for a limit. But it requires to parse command line in order to detect such cases so unit initialization code is not a good place for synchronizing.

It's exactly why I placed the comment in the code above:

Code: Select all

  {IMPORTANT: this should be called only if NOT launched as a 
   TC helper instance (Lister, Separate search, etc.), because 
   IsInstanceUnique(..,False) leaves the mutex open}

I can't agree that there is a big problem to check the command line parameters in the initialization code. This may be a simplified checking, full parsing is not required. However, this parsing should be done in a separate function, similarly to the Check function in the code above (or just inside the Check function), to ensure that all dynamically allocated strings (even those compiler-generated) will be released before the function exit - so the checking code will not interfere with any alternative memory managers, which may be installed later. See the comment in the initialization section of the UniqueInstance unit.

Problem with multiple mutexes is that we can't enumerate them so we should try opening them in some kind of loop, but at which N should it be stopped (one can start 5 instances, then close 4th, but we can't stop after 3rd, there may be 5th and other)?

This is exactly same problem as with FindWindow function - how many windows can we search for? ;) But this is rather a theoretical problem, because are we able to launch only a very limited number (from the operating system's point of view) of TC instances. I just enumerate them in the loop from 1 to High(Integer) = 2147483647. And I don't expect any problems with this in this context.


Regards!

[ghisler(Author)]

I prefer to keep using FindWindow, because I also have to enable the other program. With a Mutex, TC could still be hanging on close, and the user couldn't open another one. With FindWindow, once the window is gone, he can open a new one.

[MVV]

But I just tested this with Notepad as you suggested and I must admit, that FindWindow doesn't freeze.

I noticed today that SendMessage with WM_GETTEXT may hang while GetWindowText immediately returns text for hung window (but yes, it may be not the same as WM_GETTEXT result).

I can't agree that there is a big problem to check the command line parameters in the initialization code.

Sorry but it is ugly to insert checks everywhere just because you want to execute your code at that place instead of calling it from another one. However, both parsing command line and checking for instances should be done before form initialization.

This is exactly same problem as with FindWindow function - how many windows can we search for?

Window is a necessary object for regular TC instance while synchronization object is not necessary and every application should avoid acquiring unused kernel objects and other system resources.

[MarcinW]

every application should avoid acquiring unused kernel objects and other system resources.

But this mutex won't be unused, it will be used for "uniqueness" checking...

I prefer to keep using FindWindow, because I also have to enable the other program.

No problem, you can call FindWindow after mutex checking, activate the other TC instance and then terminate the application. Checking the mutex during the initialization will ensure, that the instance checking is reliable.

With a Mutex, TC could still be hanging on close, and the user couldn't open another one. With FindWindow, once the window is gone, he can open a new one.

Important notice. So below I attached the modified UniqueInstance.pas. Now, when destroying the main TC window, you can call:

Code: Select all

  if UniqueMutex1 <> 0 then
  begin
    CloseHandle(UniqueMutex1);
    UniqueMutex1:=0;
  end;
  if UniqueMutex2 <> 0 then
  begin
    CloseHandle(UniqueMutex2);
    UniqueMutex2:=0;
  end;

Thanks to this, invisible, terminating TC instances will not block the others.

UniqueInstance.pas:

Code: Select all

unit UniqueInstance;

interface

uses
  Windows;

var
  InstanceIsUnique : Boolean;
  InstanceNumber : Integer;
  UniqueMutex1 : THandle;
  UniqueMutex2 : THandle;

implementation

function IsInstanceUnique(const MutexName : string; CloseHandleIfNotUnique : Boolean; var MutexHandle : THandle) : Boolean;
begin
  {"The system closes the handle automatically when the process terminates."}
  MutexHandle:=CreateMutex(nil,False,PChar(MutexName));
  if MutexHandle <> 0 then
  begin
    if GetLastError = ERROR_ALREADY_EXISTS then
    begin
      Result:=False;
      if CloseHandleIfNotUnique then
      begin
        CloseHandle(MutexHandle);
        MutexHandle:=0;
      end;
    end else
      Result:=True;
  end else
  begin
    {ERROR_ACCESS_DENIED means that mutex exists}
    if GetLastError = ERROR_ACCESS_DENIED then
    begin
      Result:=False;
      if not CloseHandleIfNotUnique then
        MutexHandle:=OpenMutex(SYNCHRONIZE,False,PChar(MutexName));
    end else
      Result:=True;
  end;
end;

function Check : Boolean;
const
  MutexName = 'Unique Program Name';
var
  NumberStr : string;
  I : Integer;
begin
  InstanceIsUnique:=IsInstanceUnique(MutexName,False,UniqueMutex1);

  Result:=InstanceIsUnique;

  for I:=1 to High(I) do
  begin
    {wvsprintf, lpOut: "The maximum size of the buffer is 1024 bytes"}
    SetLength(NumberStr,1024);
    SetLength(NumberStr,wvsprintf(PChar(NumberStr),'%u',PChar(@I)));
    if IsInstanceUnique(MutexName+' '+NumberStr,True,UniqueMutex2) then
    begin
      InstanceNumber:=I;
      Break;
    end;
  end;
end;

initialization
  {We use a separate function to execute the checking code, so dynamic
  local variables, like NumberStr, will be automatically cleared before
  the function returns. Thanks to this, the checking code will not leave
  any allocated memory, so this unit can be used (listed in the unit list
  of the main .dpr file) even before memory managers, like FastMM4}
  Check;
end.

Regards