Open GoogleCodeExporter opened 8 years ago
Original comment by mirko.na...@gmail.com
on 12 Apr 2010 at 8:04
I second this! I want it too.
Original comment by shervin.asgari@gmail.com
on 13 Apr 2010 at 6:54
[deleted comment]
[deleted comment]
Here is a working patch for this issue.
The only thing I am unsure about is the PureJavaProcessManager. I had to put a
if(process != null) because I am setting null in the processManager, because I
don't know how to retrieve the Process object processManager.kill(null,
existingPid);
If you have suggestion on improvements, then please let me know
Original comment by shervin.asgari@gmail.com
on 10 Mar 2011 at 10:30
Attachments:
Yep that's precisely why implementing this feature is not trivial: there's no
pure Java way of terminating an external process.
With your suggested patch PureJavaProcessManager.kill() would do nothing,
OfficeProcess will try and start a new office process even though another one
is already running, and the app will be in an usuable state without warning the
user.
At least it should exit with an error saying that auto-killing stale office
processes is not supported when using PureJavaProcessManager.
I'm not really satisfied with the current ProcessManager implementations anyway
- they're not very robust, and it's a pain to have to support many different
versions of operating systems. But that's a separate issue...
Original comment by mirko.na...@gmail.com
on 13 Mar 2011 at 2:53
Would it be possible to create a new java process in the
PureJavaProcessManager, so that we could correctly kill that java process?
Do you want me to implement your exception suggestion and resubmit a patch?
Original comment by shervin.asgari@gmail.com
on 13 Mar 2011 at 3:54
You get a Process instance in Java only if you start a process from the same
Java app. If the process was left there by a previous run there's no (pure
Java) way you can get a handle to it.
For a related idea about process management see issue #82.
Original comment by mirko.na...@gmail.com
on 13 Mar 2011 at 4:06
FWIW, this is my workaround:
manager = configuration.buildOfficeManager();
manager.start();
Runtime.getRuntime().addShutdownHook(new Thread("JODConverterShutdownHook") {
@Override
public void run() {
manager.stop();
}
});
Original comment by bsi....@gmail.com
on 14 Mar 2011 at 8:59
That's not a workaround, that's something users should always do: make sure
stop() is called when the app terminates.
Original comment by mirko.na...@gmail.com
on 14 Mar 2011 at 9:36
Oh, I seem to have misunderstood the topic of this thread, then.
Original comment by bsi....@gmail.com
on 14 Mar 2011 at 9:50
Right, you are correct. I still have some time for this project this week.
Anything you want me to do?
Original comment by shervin.asgari@gmail.com
on 14 Mar 2011 at 9:51
I can try to refactor the code to use SIGAR if you want?
Original comment by shervin.asgari@gmail.com
on 14 Mar 2011 at 9:52
Or maybe you haven't, and other users requesting this new option are also
simply not closing their apps properly, in which case I'd be tempted to say the
new option is not needed.
Original comment by mirko.na...@gmail.com
on 14 Mar 2011 at 9:52
My last comment was in response to Comment 11.
@Shervin: yep feel free to have a look at Sigar if you like. May be better to
continue discussions on the group rather than here.
Original comment by mirko.na...@gmail.com
on 14 Mar 2011 at 9:55
I tried adding the shutdown hooks, but still it will not kill the pipes if my
application server is killed. That's why I still think its a good idea to have
the feature of autokilling the open pipes.
Mirko: I guess you haven't really decided if you want the feature in or not,
but in case of the latter, then you shouldn't have created the issue. In case
of the former, can I commit the diff in trunk?
Original comment by shervin.asgari@gmail.com
on 22 Mar 2011 at 10:41
In fact I didn't create this issue. :)
Personally I never felt the need for this feature. I've never seen an office
process still running after stopping my Java app - unless I manually kill -9
the Java app, but in that case it's pretty obvious that I need to manually kill
any process started by that app as well. And I don't want to encourage bad
practices like not closing apps properly.
That said if users keep asking for it then I can reluctantly agree to add it as
an option. That's my position.
Regarding your patch, I pointed out an issue with PureJavaProcessManager, and
the need to revise process managers anyway. Maybe you can commit the changes to
the Sigar branch, and then I'll do the merge?
Original comment by mirko.na...@gmail.com
on 22 Mar 2011 at 11:23
Sounds good. I can commit it in sigar branch.
I was thinking a little about issue 40 - WebServices. I can try to do that, but
I would like to update the pom to use Java 6, since Java 5 is EOF, and possible
Java EE 6 jaxrs restful webservices. But I don't know what your take is on that?
Original comment by shervin.asgari@gmail.com
on 22 Mar 2011 at 11:29
Code is not commited in sigar branch
Original comment by shervin.asgari@gmail.com
on 22 Mar 2011 at 11:54
Setting newly-added status "FixedInBranch" to be able to tell what still needs
to be merged into trunk.
Original comment by mirko.na...@gmail.com
on 27 Mar 2011 at 11:13
Original comment by mirko.na...@gmail.com
on 27 Mar 2011 at 1:12
Original issue reported on code.google.com by
daniel.m...@googlemail.com
on 5 Aug 2009 at 9:08