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

The local appium process does not stop with SIGINT or SIGTERM #625

Open
3 tasks done
VitaliiPivtorak opened this issue Jul 18, 2023 · 29 comments
Open
3 tasks done

The local appium process does not stop with SIGINT or SIGTERM #625

VitaliiPivtorak opened this issue Jul 18, 2023 · 29 comments

Comments

@VitaliiPivtorak
Copy link

Do I have the most recent component updates?

  • I use the most recent available driver/plugin and server versions

Is the component officially supported by the Appium team?

  • I have verified the component repository is present under the Appium organization in GitHub

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

During test run Appium generate huge amount of files *.response (like /private/var/folders/v6/fj6k1y_97yz990kh7rpjrvgm0000gp/T/8f1e5b3c-8bd6-44e6-8ddf-142f9e2537f3.response) and not deleted after. I have 240 tests which runs 3-5 times per day and on 3d day temp folder size more then 300GB.
Screen shot attached after run 3 time by 1 test.

Expected Behavior

After finish test run should be option to delete it.

Minimal Reproducible Example

Run test

Environment

  • Operating system:Mac OS 13.4.1 (c) (22F770820d)
  • Appium server version (output of appium --version):2.0.0-rc.5
  • Appium driver(s) and their version(s):- [email protected]
  • Appium plugin(s) and their version(s):
  • Node.js version (output of node --version):v18.15.0
  • npm version (output of npm --version):9.6.3
  • Last component(s) version which did not exhibit the problem:
  • Platform and version under test: Pixel 4a,5,5a Android 13
  • Real device or emulator/simulator:Real device

Link to Appium Logs

https://gist.github.com/VitaliiPivtorak/36690410feb338a012142b14f610023a

Futher Information

Screenshot 2023-07-18 at 16 24 17

@KazuCocoa
Copy link
Member

let me check tonight

@KazuCocoa
Copy link
Member

KazuCocoa commented Jul 19, 2023

How did you stop the appium process, or did you keep the appium process running?
The temp file is used to handle possible duplicated new session requests, so current implementation in appium keeps the temp file until the appium process gets stopped by SIGINT/SIGTERM.
https://github.com/appium/appium/blob/ff62e208d92995cabc35ddcc100ca4483e25b1af/packages/appium/lib/main.js#L439C26-L439C32
So, probably they will be deleted once you stop the appium process as SIGINT/SIGTERM signal. For example, when I ran an appium xcuitest session on my local appium, it generated similar naming's temp file with the new session request. Once I killed the process with SIGINT, the tmp file had gone.

I wondered how you managed appium process in your test running.

@KazuCocoa KazuCocoa removed the Bug label Jul 19, 2023
@VitaliiPivtorak
Copy link
Author

Using Nunit in OnetimeSetup i do - [OneTimeSetUp]
appiumServer =
new AppiumServiceBuilder().WithAppiumJS(new FileInfo("/usr/local/lib/node_modules/appium/build/lib/main.js"))
.WithIPAddress("127.0.0.1")
.UsingAnyFreePort()
.WithArguments(optionCollector)
.Build();
appiumServer.Start();
and in [OneTimeTearDown]
appiumServer.Dispose();

@KazuCocoa
Copy link
Member

Thank you.
It seems like https://github.com/appium/dotnet-client/blob/master/src/Appium.Net/Appium/Service/AppiumLocalService.cs#L132 (in the appiumServer.Dispose();) works as SIGKILL. Then, no resource cleaning happens. The dotnet client should kill the process as sigterm for macOS/Linux.

Lets move to dotnet client repository to explore proper way

@KazuCocoa KazuCocoa transferred this issue from appium/appium Jul 19, 2023
@KazuCocoa
Copy link
Member

@Dor-bl @laolubenson Do you know the best way to stop the local appium process with SIGINT or SIGTERM instead of SIGKILL in dotnet env? Below is the current implementation. In my research, the Kill() works as SIGKILL. It does not trigger https://github.com/appium/appium/blob/ff62e208d92995cabc35ddcc100ca4483e25b1af/packages/appium/lib/main.js#L439C26-L439C32 to cleanup resources used by Appium.

        private void DestroyProcess()
        {
            if (Service == null)
            {
                return;
            }

            try
            {
                Service.Kill();
            }
            catch (Exception ignored)
            {
            }
            finally
            {
                Service.Close();
            }
        }

https://github.com/appium/dotnet-client/blob/master/src/Appium.Net/Appium/Service/AppiumLocalService.cs#L132

@KazuCocoa KazuCocoa changed the title Appium creates a lots of temp files in "/private/var/folders/*/T/*.response" The local appium process does not stop with SIGINT or SIGTERM Jul 19, 2023
@VitaliiPivtorak
Copy link
Author

Method Dispose also call Destroy and then Kill but no delete file, server it self stoped.
private void DestroyProcess()
{
if (Service == null)
{
return;
}

        try
        {
            Service.Kill();
        }
        catch
        {
        }
        finally
        {
            Service.Close();
        }
    }

    /// <summary>
    /// Stops this service if it is currently running.
    /// </summary>
    [MethodImpl(MethodImplOptions.Synchronized)]
    public void Dispose()
    {
        DestroyProcess();
        GC.SuppressFinalize(this);
    }

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jul 19, 2023

@VitaliiPivtorak can you share where are those files located? let me check if i can try and repro from my machine

@VitaliiPivtorak
Copy link
Author

VitaliiPivtorak commented Jul 19, 2023

like /private/var/folders/v6/fj6k1y_97yz990kh7rpjrvgm0000gp/T/8f1e5b3c-8bd6-44e6-8ddf-142f9e2537f3.response
i also checkd server is run and then stoped
Screenshot 2023-07-19 at 12 25 02

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jul 19, 2023

I think I found it as well. But not a lot of files there and the biggest one is 2KB
I'll try to take a look on the code later
image

@VitaliiPivtorak
Copy link
Author

on server i have 300+ MB files in few days test run.
Screenshot 2023-07-19 at 14 53 46

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jul 19, 2023

@KazuCocoa, Do you know if it's enough to start the service? Or I should also send newSession to the driver to create that file?

@KazuCocoa
Copy link
Member

The file will be generated when you start a new session request, so start an appium process + send a new session request to the appium

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jul 20, 2023

Ok, so I have the way to repro this. but for now no luck after a couple of options I tried.
i'll keep looking into it

@KazuCocoa
Copy link
Member

Thanks. The worst case is... maybe send the signal, kill -2 <pid>, directly instead of Service.Kill() on macOS/Linux...? Win env will keep the Service.Kill.

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jul 20, 2023

thing is that also on windows those files are not cleaned up

@KazuCocoa
Copy link
Member

KazuCocoa commented Jul 20, 2023

I see... Then... not directly related for this SIGINT stuff in this ticket, but should https://github.com/appium/appium/blob/ff62e208d92995cabc35ddcc100ca4483e25b1af/packages/appium/lib/main.js#L439 have SIGHUP and SIGBREAK for Windows? Potentially this signal method does not work on Win.
https://nodejs.org/api/process.html#signal-events

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jul 20, 2023

I take it back. SIGINT works on Windows manually, Just needs to find a way to send it to the appium server via C#
image

@KazuCocoa
Copy link
Member

Do you think CloseMainWindow is more proper
https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.kill?view=net-7.0

The Kill method forces a termination of the process, while CloseMainWindow only requests a termination

I haven't checked what signal was sent on macOS, but for Win env, appium/appium#18901 and the CloseMainWindow can got to the termination hook

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jul 23, 2023

Closes a process that has a user interface by sending a close message to its main window.

What if server wasn't started from cmd, meaning no visible window?
I recall Process.CloseMainWindow won't work

@KazuCocoa
Copy link
Member

I see thanks. I thought it would not work but wanted to double check.
Maybe .net has no api to work like sigterm/sigint...? (kind of grace termination) It seems like Process.Kill and Process.Close work just kill it... (I know of ctl+c in powershell worked as sigint as you commented before)

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jul 23, 2023

I already tried 2 of the most popular solutions I found online.
generateconsolectrlevent which should generate SIGINT event
terminateprocess which should generate SIGTERM event
But none of them worked for me. :(
I did come across this one: nodejs/node#35172.
I assume we are not the first to try it out

@mykola-mokhnach
Copy link

@Dor-bl Could you also please make sure the client does not add the x-idempotency-key header to all requests, but only to the session creation ones?

The server only enables caching for POST and PATCH requests which have this header set to a non-empty string value. Check https://github.com/appium/appium/blob/master/packages/base-driver/lib/express/idempotency.js for more details

@mykola-mokhnach
Copy link

mykola-mokhnach commented Jul 29, 2023

Also for local testing you may create a simple node process containing https://nodejs.org/api/process.html#event-exit listener and then kill it using C#. If a "proper" kill method is used then the callback listener must be executed and the all good! string printed to the terminal.

Example Node.js script payload:

process.on('exit', () => {
  console.log('all good!');
});

@Dor-bl
Copy link
Collaborator

Dor-bl commented Jul 29, 2023

@Dor-bl Could you also please make sure the client does not add the x-idempotency-key header to all requests, but only to the session creation ones?

The server only enables caching for POST and PATCH requests which have this header set to a non-empty string value. Check https://github.com/appium/appium/blob/master/packages/base-driver/lib/express/idempotency.js for more details

@mykola-mokhnach Indeed we are only adding the x-idempotency-key header for NewSession commands.
But can you confirm that the header value is not Case-Senstive?
private const string IdempotencyHeader = "X-Idempotency-Key";

@mykola-mokhnach
Copy link

yes, it is. The Express framework always transforms all headers to lowercase.

@Dor-bl
Copy link
Collaborator

Dor-bl commented Aug 1, 2023

Small update:
I was able to send SIGINT to the visible console window of the running appium node:

[Appium] Welcome to Appium v2.0.1
[Appium] Attempting to load driver windows...
[debug] [Appium] Requiring driver at C:/Users/Dor-B/.appium/node_modules/appium-windows-driver
[Appium] Attempting to load driver uiautomator2...
[debug] [Appium] Requiring driver at C:\Users\Dor-B\.appium\node_modules\appium-uiautomator2-driver
[Appium] Appium REST http interface listener started on http://0.0.0.0:4723
[Appium] You can provide the following URLS in your client code to connect to this server:
[Appium]        http://10.100.102.3:4723/
[Appium]        http://127.0.0.1:4723/ (only accessible from the same host)
[Appium] Available drivers:
[Appium]   - [email protected] (automationName 'Windows')
[Appium]   - [email protected] (automationName 'UiAutomator2')
[Appium] Available plugins:
[Appium]   - [email protected]
[Appium] No plugins activated. Use the --use-plugins flag with names of plugins to activate
**[Appium] Received SIGINT - shutting down**
[debug] [AppiumDriver@8d45] There are no active sessions for cleanup
[HTTP] Waiting until the server is closed
[HTTP] Received server close event

I still need to figure out why when the node is running in the background I can't get the same result.
I created this sample repo, to test the event exit, if someone would like to take a look
https://github.com/Dor-bl/node_sigint

@Dor-bl
Copy link
Collaborator

Dor-bl commented Sep 28, 2023

@mykola-mokhnach / @KazuCocoa, can you have a look please at the provided repo?
Maybe you have an idea of how to send the event to the background node process

@mykola-mokhnach
Copy link

@mykola-mokhnach / @KazuCocoa, can you have a look please at the provided repo? Maybe you have an idea of how to send the event to the background node process

I am not a big expert there, but you could try the solution provided in https://stanislavs.org/stopping-command-line-applications-programatically-with-ctrl-c-events-from-net/

What's important:

Pinvoke only: Start with invisible window, get window handle, run for 6 seconds, stop using window handle. (Unfinished!)

It turned out that it was impossible to start a command window with an invisible, but still existing window using pinvoke CreateProcess. The start sequence ended up being the same as for Scenario 1, but bypassing .Net. Stopping a process with PostMessage did not work out either, even though I read of successful attempts. I abandoned this scenario as I did not want to use an inordinate amount of time on what started to look like a dead end.

So you probably should stick to the scenario 1:

Start with a visible window using .Net, hide with pinvoke, run for 6 seconds, show with pinvoke, stop with .Net.

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

No branches or pull requests

4 participants