Skip to content
New issue

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

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

Already on GitHub? # to your account

Cleanup routine throws ArgumentException #33

Closed
naspert opened this issue Jan 17, 2017 · 9 comments
Closed

Cleanup routine throws ArgumentException #33

naspert opened this issue Jan 17, 2017 · 9 comments
Assignees

Comments

@naspert
Copy link

naspert commented Jan 17, 2017

Calling Process.GetProcessById throws if the process with the given id is not running.
cf.
https://github.com/oleg-shilo/cs-script/blob/master/Source/Utils.cs#L288

wrapping the call inside a try/catch block should do the trick...

@naspert
Copy link
Author

naspert commented Jan 17, 2017

@oleg-shilo
Copy link
Owner

oleg-shilo commented Jan 18, 2017

Not sure I follow you. It is already wrapped. I the very same source code link you provided::

try
{
    if (Process.GetProcessById(pid) != null)
        continue; //still running
}
catch
{
    //GetProcessById will throw if pid is not running and Utils.FileDelete handles all unexpected
}

@naspert
Copy link
Author

naspert commented Jan 18, 2017

true I missed that. However this behavior has changed wrt previous versions, increasing the noise level when debugging. If I set the environment var "CSScript_Suspend_Housekeeping" I should avoid entirely this routine, right ? (currently tracing an issue at when my application exits and CSScript exceptions get in the way...)

@naspert
Copy link
Author

naspert commented Jan 18, 2017

(replying to myself) with the env. variable properly set, I managed to trace the real problem. I will close the issue (but still a non-throwing version would be a nice feature ;)

@naspert naspert closed this as completed Jan 18, 2017
@oleg-shilo
Copy link
Owner

Well... I agree. I also found it extremely annoying when I have to step over handled exceptions while hunting for a real ones.
I didn't want to replace it with the all process lookup by id unless there is a good reason for this. But since you raised this issue I will do the replacement.

@oleg-shilo oleg-shilo reopened this Jan 18, 2017
@naspert
Copy link
Author

naspert commented Jan 18, 2017

I guess you can replace this call by something like this (which should not throw)
var tmp = Process.GetProcesses().Select(p => p.Id);
...
and
if (tmp.Contains(pid)) continue;

@oleg-shilo
Copy link
Owner

Of course. It is exactly what I am going to do. Another good candidate is this:

if (Process.GetProcesses().Any(p => p.Id == pid))
    continue; //still running

Another reason for not doing it earlier is that .NET 1.1 didn't have LINQ and foreach... really hurts my eyes. :) Yes, yes I know, foreach is the fastest option but it.... just kills the fun. :)

But now support for 1.1 is over so many things are becoming much simpler.

@oleg-shilo
Copy link
Owner

Fixed in 58ce13f
Planned for v3.21 release

@oleg-shilo oleg-shilo self-assigned this Jan 19, 2017
oleg-shilo added a commit that referenced this issue Jan 25, 2017
* Added support for C#6 syntax to Mono evaluator.
* Issue #33: Cleanup routine throws ArgumentException
* Added extending environment variables in all parameters passed from command line.
* Improved API XML documentation to address some of the Issue #26 concerns.
* User experience improvements triggered by incentive Linux testing
 * Various improvements for stdout help.
 * Improved reliability of Auto-class decorating algorithm
 * Added support for referencing NuGet packages from the script being executed on Linux
 * Added -noconfig:print and -precompiler:print options for printing the content in stdout.
* Extended Python-like "print" functionality:
 * Added params concatenation: `print(obj1, obj2,...objN)`
 * Added pringf for params formatting: `printf("Now: {0}", DateTime.Now)`
 * added support for collections: `print(Directory.GetFiles(".", "*"))`
 * Added decorateAutoClassAsCS6 setting for injection `using static dbg;` into auto-class decoration.
* Issue#32: Inconsistent time stamp (.dll -vs- .pdb) when using `CSScript.CompileFile()` with 'debugBuild = true'
* Added support for output file in //css_res.
* Various obsolete code marked as error triggering.
* Implemented supressing elevation during syntax checking (with `-check`) for the scripts with `//css_pre elevate` directive.
* Improved settings file parsing to avoid throwing handled exceptions.
* Code cleanup
 * Added support for CSS_RESGEN environment variable for embedding resources with //css_res.
 * Removed old obsolete ResolveSourceFileHandler delegate and MonoEvaluator.Configuration member.
 * Staretd removal of obsolete .NET1.1 code (conditional compiler directives).
@oleg-shilo
Copy link
Owner

Fixed in Release v3.21.1

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants