Details

    • Type: Bug Bug
    • Status: Reopened
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.3
    • Fix Version/s: None
    • Component/s: Collection Processing
    • Labels:
      None

      Description

      If a daemon thread was started within an AE and keeps running independent of the AE processing (e.g. bound to a static variable) it prevents exiting of the CPE.

      Simple test:
      add following class to a AE:

      static class DaemonThread implements Runnable {
      	private static volatile Thread threadInstance;
      	/** Creates a daemon thread if it not already exists. */
      	public synchronized static void startThread() {
      		if ( threadInstance == null ) {
      			threadInstance = new Thread( new DaemonThread(), "TestDaemon" );
      			threadInstance.setDaemon( true );
      			threadInstance.start();
      		}
      	}
      	@Override
      	public void run() {
      		while ( true ) {
      			try { Thread.sleep( 2000 ); } catch ( InterruptedException ie ) {}
      		}
      	}
      }
      

      and call DaemonThread.startThread(); within the process() method.

      After CPE signals collectionProcesscomplete() the application does not exit.
      The reason: in org.apache.uima.collection.impl.cpm.engine.CPMEngine a threadGroupDestroyer thread is created (line 2522) which waits for threads to die. Since it does not check for daemon threads it counts them as normal threads to wait for and thus it waits forever (this can be seen with kill -3 PID to get a JAVA stack trace of all threads under UNIX).

      A workaround would be to put the daemon thread in another ThreadGroup. In the example if you create the tread with

      			ThreadGroup parent = Thread.currentThread().getThreadGroup();
      			while ( parent.getParent() != null )
      				parent = parent.getParent();
      			threadInstance = new Thread( parent, new DaemonThread(), "TestDaemon" );
      

      (in the top most thread group) the CPE exits normally.
      However this is not possible with closed source libraries.

      Thus the threadGroupDestroyer must test for daemon threads. Furthermore the used group.activeCount() (in org.apache.uima.collection.impl.cpm.engine.CPMEngine) should only be used for information purposes - as the javadoc states - not for relying on it as a loop parameter.

      1. CPMEngine.diff
        5 kB
        Timo Boehme
      2. cpm_messages.diff
        0.7 kB
        Timo Boehme

        Activity

        Hide
        Richard Eckart de Castilho added a comment -

        Sounds good to me.

        Show
        Richard Eckart de Castilho added a comment - Sounds good to me.
        Hide
        Marshall Schor added a comment -

        This sounds reasonable to me. Any other views?

        Show
        Marshall Schor added a comment - This sounds reasonable to me. Any other views?
        Hide
        Timo Boehme added a comment -

        Typically we run into the daemon thread issue when data resources are initialized lazily like a database access only needed at specific circumstances. It is common with some database drivers (e.g. Derby in application mode) that a daemon thread is started for monitoring/cleanup work which is still running even when the connection is closed. Since these threads are marked daemon it is supposed that they should not block a system shutting down.

        The patch adds very little overhead to the current implementation since thread enumeration was already called (even if the information wasn't used at all). It uses a defined maximum time span to allow for daemon threads to shutdown (after all other non-daemon threads stopped). If after this time there are still daemon threads it at least sets the group to be a daemon group, signaling the Java VM to release resources as soon as possible.
        The main difference to the previous implementation is that it now won't block forever but allows the CPE to shutdown if only daemon threads are left.

        I would appreciate it if the patch or a similar working solution could be added to UIMA. This would facilitate a more predictable shutdown behavior, especially if 3rd party software is integrated which might run daemon threads and cannot be controlled in how this is done.

        Show
        Timo Boehme added a comment - Typically we run into the daemon thread issue when data resources are initialized lazily like a database access only needed at specific circumstances. It is common with some database drivers (e.g. Derby in application mode) that a daemon thread is started for monitoring/cleanup work which is still running even when the connection is closed. Since these threads are marked daemon it is supposed that they should not block a system shutting down. The patch adds very little overhead to the current implementation since thread enumeration was already called (even if the information wasn't used at all). It uses a defined maximum time span to allow for daemon threads to shutdown (after all other non-daemon threads stopped). If after this time there are still daemon threads it at least sets the group to be a daemon group, signaling the Java VM to release resources as soon as possible. The main difference to the previous implementation is that it now won't block forever but allows the CPE to shutdown if only daemon threads are left. I would appreciate it if the patch or a similar working solution could be added to UIMA. This would facilitate a more predictable shutdown behavior, especially if 3rd party software is integrated which might run daemon threads and cannot be controlled in how this is done.
        Hide
        Timo Boehme added a comment -

        Patch which adds special handling when only daemon threads are left in CPM thread group and system is shutting down.
        The patch will wait a specified maximum time (currently 10 seconds; this could be made adjustable) if only threads with daemon state are left in the CPM thread group. After this time the group is set itself to be 'daemon' (enabling Java to release resources as early as possible) and a warning is logged that the group cannot be destroyed.

        Show
        Timo Boehme added a comment - Patch which adds special handling when only daemon threads are left in CPM thread group and system is shutting down. The patch will wait a specified maximum time (currently 10 seconds; this could be made adjustable) if only threads with daemon state are left in the CPM thread group. After this time the group is set itself to be 'daemon' (enabling Java to release resources as early as possible) and a warning is logged that the group cannot be destroyed.
        Hide
        Timo Boehme added a comment -

        I hope re-opening an issue is ok with you. I'm doing it for the following reasons:

        • the issue is still true with the current trunk
        • the possibilities for having daemon threads left in the CPM thread context
          are much broader than mentioned in above comment by Marshall Schor
        • now I have a patch to be provided
        Show
        Timo Boehme added a comment - I hope re-opening an issue is ok with you. I'm doing it for the following reasons: the issue is still true with the current trunk the possibilities for having daemon threads left in the CPM thread context are much broader than mentioned in above comment by Marshall Schor now I have a patch to be provided
        Hide
        Marshall Schor added a comment -

        I'm planning on closing this without fixing anything, for the following reasons:

        • There are other conditions (such as starting a non-daemon thread inside a process method) that would prevent exit.
        • A somewhat more general way to handle this that I think works is to implement a destroy() method for the Annotator whose process starting these extra threads, and have it do whatever reasonable cleanup might be appropriate, including signalling the threads to terminate themselves (daemon or not).
        • It's somewhat unusual to have threads started in the process method - we want to have the framework handle the more frequent use-cases, and not put in too much special handling for edge cases.
        Show
        Marshall Schor added a comment - I'm planning on closing this without fixing anything, for the following reasons: There are other conditions (such as starting a non-daemon thread inside a process method) that would prevent exit. A somewhat more general way to handle this that I think works is to implement a destroy() method for the Annotator whose process starting these extra threads, and have it do whatever reasonable cleanup might be appropriate, including signalling the threads to terminate themselves (daemon or not). It's somewhat unusual to have threads started in the process method - we want to have the framework handle the more frequent use-cases, and not put in too much special handling for edge cases.
        Hide
        Timo Boehme added a comment -

        In one or two weeks I may get time to provide a patch for this.

        Show
        Timo Boehme added a comment - In one or two weeks I may get time to provide a patch for this.
        Hide
        Marshall Schor added a comment -

        Thanks, Timo. Can you provide a patch for this?

        Show
        Marshall Schor added a comment - Thanks, Timo. Can you provide a patch for this?

          People

          • Assignee:
            Unassigned
            Reporter:
            Timo Boehme
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:

              Development