SneakyLaiji / jodconverter

Automatically exported from code.google.com/p/jodconverter
0 stars 0 forks source link

WindowsProcessorManager hangs after NPE when JodConverter run as an unprivileged user #49

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Become an unprivileged user inside Windows XP (Professional, SP3)
2. Run JodConverter against a document that will hang OO.org (in my case,
one with an embedded graph object)
3. Wait for timeout
4. Observe exception:

Caused by: org.artofsolving.jodconverter.office.OfficeException: could not
termi
nate process
        at
org.artofsolving.jodconverter.office.ManagedOfficeProcess.doTerminate
Process(ManagedOfficeProcess.java:155)
...
Caused by: java.lang.NullPointerException
        at java.lang.ProcessBuilder.start(Unknown Source)
        at
org.artofsolving.jodconverter.process.WindowsProcessManager.execute(W
indowsProcessManager.java:51)
        at
org.artofsolving.jodconverter.process.WindowsProcessManager.kill(Wind
owsProcessManager.java:37)
        at
org.artofsolving.jodconverter.office.OfficeProcess.forciblyTerminate(
OfficeProcess.java:185)
        at
org.artofsolving.jodconverter.office.ManagedOfficeProcess.doTerminate
Process(ManagedOfficeProcess.java:152)
        ... 9 more

5. Process hangs

Note that this works fine when running under XP Professional as an
administrator user. 

The problem is that WindowsProcessManager is executing wmic to find the
process ID for OpenOffice, and then using taskkill to kill the process with
that process ID. wmic cannot be run by an unprivileged user (although
oddly, taskkill can be). So, the output of wmic is a "this cannot be run"
message, and the findPid method can't find a PID in that, so returns null,
hence the NPE later on.

The solution is to change taskkill to kill the process directly by name,
rather than using a process ID lookup.

In WindowsProcessManager, change line 37:
  execute("taskkill", "/t", "/f", "/pid", pid);

to
  execute("taskkill", "/t", "/f", "/im", "soffice.bin");

Remove lines 21-32, so the body of findPid is just "return null". It's not
needed on Windows in any case.

Original issue reported on code.google.com by inigo.su...@gmail.com on 15 Jul 2009 at 2:13

GoogleCodeExporter commented 8 years ago
Incidentally, taskkill won't work on older versions of Windows, or on XP Home, 
as is
noted in the comments. The equivalent for Windows 2000 and above is tkill, 
which has
the syntax "tkill soffice". This is available on Windows XP, but isn't 
available on
all versions of Windows Vista.

Original comment by inigo.su...@gmail.com on 15 Jul 2009 at 2:26

GoogleCodeExporter commented 8 years ago
Thanks for reporting this issue.

Your proposed solution unfortunately wouldn't work if there are multiple OOo 
processes.

WindowsProcessManager#isUsable() does check if wmic and taskkill are available. 
If
they're not, then PureJavaProcessManager is used instead. (So Home editions are 
not a
problem.)

We can try and modify isUsable() to check that the wmic output is not "cannot be
run". Failing that, we'll need to always default to PureJavaProcessManager on 
Windows.

Original comment by mirko.na...@gmail.com on 16 Jul 2009 at 10:33

GoogleCodeExporter commented 8 years ago
See also issue #82

Original comment by mirko.na...@gmail.com on 13 Mar 2011 at 4:01

GoogleCodeExporter commented 8 years ago
It's interesting to see what this behaviour will be when using Sigar. But you 
will get access denied in Sigar as well and thus perhaps the same behaviour

Original comment by shervin.asgari@gmail.com on 23 Mar 2011 at 10:58

GoogleCodeExporter commented 8 years ago
My hope is that using Sigar will solve this sort of issues; in fact that's one 
of the reason I suggested looking at Sigar.

A user may not have permission to execute wmic.exe, but it should still be able 
to make native calls to find/kill processes started by the same user.

Original comment by mirko.na...@gmail.com on 23 Mar 2011 at 12:35

GoogleCodeExporter commented 8 years ago
WindowsProcessManager has now been removed in favour of SigarProcessManager - 
see issue #82

Original comment by mirko.na...@gmail.com on 3 Apr 2011 at 1:07