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

Pass thread priority setting into NameableThreadFactory constructor #190

Closed
ghost opened this issue Oct 24, 2022 · 1 comment
Closed

Pass thread priority setting into NameableThreadFactory constructor #190

ghost opened this issue Oct 24, 2022 · 1 comment

Comments

@ghost
Copy link

ghost commented Oct 24, 2022

We've hooked into the D-Bus using 4.2.1 as follows:

return DBusConnectionBuilder
        .forSessionBus()
        .receivingThreadConfig()
        .withSignalThreadPriority(MIN_PRIORITY)
        .withRetryHandler((t, ex) -> {
            // A NullPointerException occurred; the JVM has entered an irrecoverable state.
            new ShutdownEvent(ex).publish();

            // Terminate the D-Bus thread's retry handler.
            return false;
        })
        .connectionConfig()
        .build();

The call to withSignalThreadPriority performs:

config.setSignalThreadPriority(Util.checkIntInRange(_priority, Thread.MIN_PRIORITY, Thread.MAX_PRIORITY))

That call assigns the thread priority:

void setSignalThreadPriority(int _signalThreadPriority) {
  signalThreadPriority = _signalThreadPriority;
}

The signal thread priority doesn't appear to be used because the getSignalThreadPriority() method isn't invoked.

Within the constructor for ReceivingService, a new NameableThreadFactory instance is added to the internal executors map:

ReceivingService(ReceivingServiceConfig _rsCfg) {
  ReceivingServiceConfig rsCfg = Optional.ofNullable(_rsCfg).orElse(ReceivingServiceConfigBuilder.getDefaultConfig());
  executors.put(ExecutorNames.SIGNAL, Executors.newFixedThreadPool(rsCfg.getSignalThreadPoolSize(), new NameableThreadFactory("DBus-Signal-Receiver-", true)));

The ReceivingService invokes the two-argument constructor for NameableThreadFactory, which uses a default priority of Thread.NORM_PRIORITY:

public NameableThreadFactory(String _name, boolean _daemonizeThreads) {
  this(_name, _daemonizeThreads,Thread.NORM_PRIORITY);
}

Does the ReceivingService constructor need to call a different NameableThreadFactory constructor? That is, the one that takes the thread priority? Such as by calling _rsCfg.getSignalThreadPriority():

ReceivingService(ReceivingServiceConfig _rsCfg) {
  ReceivingServiceConfig rsCfg = Optional.ofNullable(_rsCfg).orElse(ReceivingServiceConfigBuilder.getDefaultConfig());
  executors.put(ExecutorNames.SIGNAL, Executors.newFixedThreadPool(
    rsCfg.getSignalThreadPoolSize(),
    new NameableThreadFactory(
      "DBus-Signal-Receiver-",
      true,
      _rsCfg.getSignalThreadPriority()))); // Is this parameter needed?

Also, do we need to pass in the priority for the sending side? That is, in AbstractConnection:

  receivingService = new ReceivingService(_rsCfg);
  senderService =
    Executors.newFixedThreadPool(1, new NameableThreadFactory("DBus Sender Thread-", false));

Does that need to pass _rsCfg.getSignalThreadPriority() as well?

@hypfvieh
Copy link
Owner

Thanks for reporting and explaining everything so well. I appreciate that.
But providing a PR to fix these issues would be much more appreciated.
If the PR is fine, I will simply merge it, if there is something I don't like we can discuss that.

This would spare me some time fixing all of that by myself.
Even if the solution is easy, I think you should take the credit for fixing because you found the problem.

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

1 participant