On Fri, 2 Apr 2021 20:27:04 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> I'm unsure of your motivation for all the work put into this issue.
When I modified LinuxDebuggerLocal to add ptrace accessor, I couldn't
understand why the method should be qualified `synchronized`. ptrace caller
should only one thread, so I understand the need of worker thread.
The comment of WorkerThreadTask says the need of worker thread, but it does not
mention the caller should be synchronized.
We can just add the comment of course, but we can simplify the worker with
modern API. We might add new worker tasks in future, so I thought it's better
to refactor in this opportunity.
For example, following the current approach, we need to add new worker task in
below:
class CreateDwarfParserTask implements WorkerThreadTask {
DwarfParser result;
public void doit(LinuxDebuggerLocal debugger) {
result = new DwarfParser(libptr, rip, rbp, rsp, debugger);
}
}
CreateDwarfParserTask task = new CreateDwarfParserTask();
workerThread.execute(task);
return task.result;
But we can simplify it after this PR:
try {
return ptraceWorkerThreadPool.submit(() -> new DwarfParser(libptr, rip,
rbp, rsp, LinuxDebuggerLocal.this))
.get();
} catch (InterruptedException | ExecutionException e) {
throw new DebuggerException(e);
}
This PR will take a lot of reviewing time as you said. If it does not worth to
work, I want to add the comment about `synchronized` at least.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3321
