JSword
  1. JSword
  2. JS-237

Installer should allow external thread pool/factory

    Details

    • Type: Bug Bug
    • Status: Closed (View Workflow)
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.7
    • Component/s: None
    • Labels:
      None

      Description

      Since some tests depend on having some modules installed on the target, I've put together a simple programmatic install step to be run during tests (to ensure we have the artifacts we need).

      the problem is that I need to know when the install process finishes, and ideally, have an optional blocking operation for a book install process.

      This can be partially solved by putting some method on the installer which join() all the worker threads, but ideally, I should be able to specify an external threadpool/worker.

        Activity

        Hide
        Chris Burrell added a comment -

        +1 I could do with a synchronous way of doing this. join() wasn't an option for me, as the worker threads are never returned to the caller. I've had to look up the progress message and determine from that whether it's finished. Or do a thread.sleep to wait for status of book to be installed.

        Show
        Chris Burrell added a comment - +1 I could do with a synchronous way of doing this. join() wasn't an option for me, as the worker threads are never returned to the caller. I've had to look up the progress message and determine from that whether it's finished. Or do a thread.sleep to wait for status of book to be installed.
        Hide
        DM Smith added a comment -

        Have you looked at o.c.j.bridge.BookInstaller for example code.

        Recently fixed a bug in there that had the same problem that you were describing. As it was a contributed fix, I'm not certain that you can catch the end. But I think so.

        Show
        DM Smith added a comment - Have you looked at o.c.j.bridge.BookInstaller for example code. Recently fixed a bug in there that had the same problem that you were describing. As it was a contributed fix, I'm not certain that you can catch the end. But I think so.
        Hide
        DM Smith added a comment -

        It was BookIndexer to which the fix was applied. Should be similar issue here.

        Show
        DM Smith added a comment - It was BookIndexer to which the fix was applied. Should be similar issue here.
        Hide
        Chris Burrell added a comment -

        The issue here is two-fold:

        • being able to install a book in a synchronous fashion
        • being to specify an external thread-pool

        On the first, that's what we need to do for tests for example. On the second, this around design, in that it is usually frowned upon to create your own threads in a J2E container (say Tomcat, JBoss, WebLogic, etc.). Generally speaking the container should be managing the threads.

        Ideally the functionality in JSword should usually/always be written in a synchronous fashion, with a set of utilities on the side that allow you to do things in the background asynchronously and an optional thread pool to do this work rather than using JSword to create the threads.

        Show
        Chris Burrell added a comment - The issue here is two-fold: being able to install a book in a synchronous fashion being to specify an external thread-pool On the first, that's what we need to do for tests for example. On the second, this around design, in that it is usually frowned upon to create your own threads in a J2E container (say Tomcat, JBoss, WebLogic, etc.). Generally speaking the container should be managing the threads. Ideally the functionality in JSword should usually/always be written in a synchronous fashion, with a set of utilities on the side that allow you to do things in the background asynchronously and an optional thread pool to do this work rather than using JSword to create the threads.
        Hide
        Chris Burrell added a comment -

        Is it perhaps worth investigation using JSR236 and JSR237 to implement this framework. http://commonj.myfoo.de/ gives a tiny implementation of it.

        Show
        Chris Burrell added a comment - Is it perhaps worth investigation using JSR236 and JSR237 to implement this framework. http://commonj.myfoo.de/ gives a tiny implementation of it.
        Hide
        DM Smith added a comment -

        +1 We should get rid of threads in JSword. Need to work with front-ends to make it happen gracefully. Maybe keep the old marking it deprecated.

        Show
        DM Smith added a comment - +1 We should get rid of threads in JSword. Need to work with front-ends to make it happen gracefully. Maybe keep the old marking it deprecated.
        Hide
        DM Smith added a comment -

        It was trivial to remove thread creation in the call an move it to the caller.

        Show
        DM Smith added a comment - It was trivial to remove thread creation in the call an move it to the caller.
        Hide
        DM Smith added a comment -

        Both Installer and Indexer are now synchronous. Threading is the caller's responsibility.

        Show
        DM Smith added a comment - Both Installer and Indexer are now synchronous. Threading is the caller's responsibility.

          People

          • Assignee:
            DM Smith
            Reporter:
            Douglas Campos
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: