Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#117 Reworked Stop.bat (used java instead of telnet and cscript) #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dmytro-sheyko
Copy link

Stop.bat now calls new java class ibcalpha.ibc.IbcStop the same way as ibcalpha.ibc.IbcTws and ibcalpha.ibc.IbcGateway are called by StartTWS.bat and StartGateway.bat.
ibcalpha.ibc.IbcStop in turn reads parameters CommandServerPort and BindAddress from config.ini.
Besides reworking Stop.bat there are some slight improvements (I believe):

  • got rid of HOMEDRIVE and HOMEPATH env vars where they mentioned;
  • env var IBC_PATH is deduced from the path of the current script. It can be handy when IBC is installed in non-standard directory (i.e not in C:\IBC);
  • introduced env var PRESS_ANY_KEY_ON_ERROR, which controls whether or not Press any key to close this window is necessary. Waiting for user input is not desiable in non-interactive mode.

@rlktradewright
Copy link
Member

Sorry for the delay in responding to this. I have actually been looking at it now and again over the past few days, and finally decided to write some comments last night.

But before doing so, I thought I should just test it to make sure the STOP entry point actually works, and immediately I realised the problem with this whole approach that blows it out of the water.

The computer I do my IBC development on is not the same one that I run TWS/Gateway on (except at weekends when my live and paper TWS/Gateway are not in use). So when I decided to use your revised STOP.bat, it was obviously untenable because it depends on being on the same machine as TWS - either that or at least having IBC installed and the the same config.ini (and a suitable Java).

Since I would imagine that by far the most common use of the STOP command is to shut down a remote instance of IBC, it is clearly unacceptable to have to duplicate the IBC configuration on the machine where STOP is to be used.

So this is not going to fly.

What we actually need is a free-standing program that has address and port number as arguments (defaulting address to 127.0.0.1 and port to 7462), and doesn't have any dependencies on IBC itself. There would then actually be no need for a STOP.bat, though one could still be provided for those who want a one-liner using non-default parameters but who really don't want to learn how to do .bat files.

I suggest that the sensible approach to this is to write a small .Net command line program (C# preferred), As for your Java version, it would only be a few lines long. Now that .Net is included in all (supported) Windows versions there would be no need to install anything else. The STOP program could simply be copied to any machine, or downloaded direct from GitHub onto any machine.

If this program were targeted on .Net Standard 2, it would also be theoretically useable cross-platform, though this would require the .Net runtime (and the SDK to allow building) to be installed on Linux and is of little real value since telnet already does the job properly.

By the way, you seem to have misunderstood the meaning of the BindAddress setting in config.ini. You can't use this setting as the address for connection to the command server, because it's rarely (if ever) set. As the comment in config.ini says, it's only relevant where the computer is multihomed (ie has more than one network adapter) but commands are only to be accepted on one of them: possibly not used by anyone, but it has been there since before I took over development of IBController in 2003.

@dmytro-sheyko
Copy link
Author

dmytro-sheyko commented Jul 20, 2021

By the way, you seem to have misunderstood the meaning of the BindAddress setting in config.ini.

Ok. Let me explain my intention. Most likely BindAddress is not specified, this means that the command server accept connections from any network adapters. In this case we can connect to the command server using "localhost" (or 127.0.0.1). In some cases BindAddress is set to 127.0.0.1 in order to avoid connection from other hosts. In this case connection to "localhost" will also work. In some very rare cases BindAddress can be set to something different, let's say 192.168.10.20 (assuming that 192.168.10.20 is a valid IP address of one of network adapters of the host). In this case an attempt to connect to the command server using conventional "localhost" will NOT work. So I was trying to cover such cases as well using BindAddress as a hint which IP address we can use in order to reach the command server. Perhaps I should have explained this in comments.

Also I was trying to improve existing Stop.bat, provided along with StartXXX.bat scripts and assumed as "official" way to stop IBC. We use Start/Stop scripts from Task Scheduler and therefore they are executed on the same host. In this case it looks unnecessary to configure Stop.bat independently from the other part of IBC. Why we should specify CommandServerPort and ServerAddress in Stop.bat if we can take them from config.ini? Please note that this also means that we should remember to update these parameters in both places, if we have to do this.

However I agree with you that for use case when Stop.bat is executed from the other host, it would be very undesirable to duplicate whole IBC and config.ini. And small standalone program would be more sensible.

@rlktradewright
Copy link
Member

Ok, I understand where you're coming from, and why your PR works in the limited context of localhost.

But please explain to me why you use Stop.bat at all if you're only running it from Task Scheduler on the same machine? Why don't you just set the auto-logoff time in TWS/Gateway? Or even use the ClosedownAt setting in config.ini?

It's not correct to say that Stop.bat is the "official" way to stop IBC. That's not really a meaningful term anyway, but the natural way to stop IBC is by the user simply exiting from TWS/Gateway. The fact is that the STOP command (ie to the command server) was originally provided as a way to shut down TWS/Gateway tidily when it is under the management of another program, and indeed there is a sample C# program (that's not currently in the IBC repository) that runs IBC as a service, enabling it to be started and stopped using the Services MMC.

The STOP command also happens to be a convenient way to shut down IBC when it's running on a remote server and you can't get to the UI without a bit of hassle: and of course a prime example of that is if you start IBC from a scheduled task configured to run even if the user is not logged on, because then you can't get to the UI at all (but of course setting the auto-logoff time or ClosedownAt still work fine, so you still only need the STOP command if you want to stop it at some other time).

Sorry, I'm out of time - there is more to say here, but I'll have to continue later...

@rlktradewright
Copy link
Member

Continuing with this...

The fact that your PR is limited to local usage doesn't mean it's of no value. It would still be helpful to provide an easy way to use the STOP command in a local context, especially since a frequent usage might be the not-logged-in task scheduler scenario where the current STOP.bat doesn't work at all.

So here are some detailed comments on your proposal, in no particular order:

  1. I'd prefer to keep this PR focussed entirely on the STOP mechanism, so please remove updates to the scripts that aren't specifically related to that, and put them in a separate PR: for example set IBC_PATH=%~dp0., set PRESS_ANY_KEY_ON_ERROR=1, use of USERPROFILE, etc.

  2. I would like to see it 'packaged' differently. It seems very odd to me to have a new STOP.bat that merely repeats the information that's already in StartTWS.bat and StartGateway.bat. I think a better approach would be to simply allow an action parameter to be supplied to the StartXXX scripts, with permitted values of start and stop (default start). So you run TWS with StartTWS and you stop it with StartTWS stop.

    With that change, we can then keep the current STOP.bat until we have a telnet-replacement program as mentioned earlier. Although STOP.bat is a mess, it does actually work apart from the not-logged-in task scheduler case, so I'm relaxed about not doing this immediately.

    I realise that this might be slightly confusing, because if you have both StartTWS.bat and StartGatway.bat configured the same, either one can then be used to stop TWS or Gateway. That's why a separate STOP script is 'nicer'. So an alternative would be to keep your new STOP.bat, rename it to STOPLOCAL.bat, and remove all the unnecessary stuff: the only set statements needed are for:

   TWS_MAJOR_VRSN
   CONFIG
   IBC_PATH
   TWS_PATH
   LOG_PATH
   JAVA_PATH

so all the other settings and corresponding notes can be removed.

  1. Further to this (and regardless of whether we go for a STOPLOCAL.bat or just add action arguments), I'd much prefer if the STOP entrypoint did only what is necessary and nothing more. The log file for STOP is full of pointless junk which is really only there because that was the easiest thing to do. In particular:

    • there's not much point in displaying the banner. It would only be useful at all if there's an error. Otherwise it appears and disappears so quickly that it's pointless

    • is it worth logging the arguments?

    • the classpath only needs to include IBC.jar

    • the VM options are irrelevant

    • the STOP entrypoint should only expect one argument

    • the list of IBC settings adds nothing

    • the actual java command to run STOP would preferably look something like this:

      "c:\users\rlk\.i4j_jres\1.8.0_152-tzdata2019c_64\bin\java.exe" -cp "C:\IBC\.\IBC.jar" ibcalpha.ibc.IbcStop "C:\Users\rlk\Documents\IBC\config.ini"
  1. The IbcStop entrypoint needs to cater for the case where the Settings class is injected rather than using the DefaultSettings class. The samples\Ibcloader folder contains a simple example that does this. Just add a public static void stop() method that can be called by the external program after it has injected its dependencies.

  2. The log file contains this near the beginning: "Starting IBC version ... ". Should be "Stopping..."

  3. The STOP.bat file contains Unix line endings. This is not a problem on recent Windows versions, but it is on Windows Server 2016 if you try to edit it with Notepad.

  4. I installed your version on the computer where I run TWS, and ran the updated STOP.bat. Sometimes it worked, but on other occasions the IBC for STOP logged 'STOP command is sent successfully', but the IBC for TWS logged an exception on trying to read the command because the client had already disconnected (ie it never got as far as receiving the STOP command at all). You need to wait for the response from the TWS IBC before closing the STOP IBC.

.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants