JKISoftware / JKI-State-Machine-Objects

Object-oriented framework for LabVIEW based on the JKI State Machine
BSD 3-Clause "New" or "Revised" License
96 stars 55 forks source link

Best pratice for dependency init/start/stop/destroy #8

Closed 3esmit closed 8 years ago

3esmit commented 8 years ago

I can start dependencies of actors in various ways.

Nested dependency management:

Where to do this:

Or simply:

What is working best for me is the "main SMO" managing all dependencies and by overwriting the corresponding action vi.

3esmit commented 8 years ago

This is what I currently, I call this VI in onCreate. On start is just the same vi but with 'Start.vi's. Stop just inverts the sequence (stop first dependents and for last the dependencies)

image

francois-normandin commented 8 years ago

This is a very interesting topic, and there are no unique answers. Your description of what works best for you is one of the patterns I used early on with SMOs. I like to have my dependencies well encapsulated if they are an integral part of an object. Some other times, a system might be decorated with different interfaces depending on configuration. I personally decide on which pattern I use based on which object is responsible for the lifetime of the subcomponent, and how likely I am to expose the subcomponent's functionalities to the outside.

My favorite pattern is to use the "onAction" methods, and particularly to use onStart and onStop, rather than onCreate and onDestroy. This composition pattern emerges because we like to use configuration-driven dependency injection. The reasoning is that configuration of the parent object is loaded after the object is created but before the object is started. Once my object has its configuration, it can create and start its necessary subparts using "onStart". Similarly, if my configuration changes, the object can be stopped along with the subparts using "onStop", and restarted with new parameters: my subparts might be of different type, so I choose not to create it until then. This pattern is useful for abstracting highly-configurable reuse components

Another pattern that seems popular is to create all the objects on the top-level VI and inject them in our objects using a public "Set Dependencies" method that is specific to the child classes. This static dependency injection should occur after the parent object is created, but before it is started. As an aggregation pattern, it is helpful if one wants to see the whole subset of dependencies and relationships. This pattern provides an easier point-of-entry into a project for teams where multiple developers with different technical levels need to quickly be able to jump in and debug/code on someone else's architecture. It is not as flexible, but it is more intuitive to see the object relationships on a flat block diagram.

Finally, I'd like to point that there are some changes introduced in version 1.1 (package to be published soon) that implement a native SMO dependency support for both composition and aggregation patterns. The code is already in the master branch if you would like to have a look. Check the example "src/Test-SMObase.vi" for a snippet of how it works. There will be soon a template added to the template package that uses this pattern.

I'm really eager to see more feedback on which design patterns other people come up with to solve their problem. Thanks for contributing!

3esmit commented 8 years ago

As you mentioned, the "Set Dependencies" is what I'm using as it the most easy to undestand what's happening and to distribute the dependencies, but I would prefer to encapsulate them. And SMO Editor Tool with dependency is a great improvement!

3esmit commented 8 years ago

As I attach more components that VI gets pretty artistic, I mean, complicated... image

3esmit commented 8 years ago

I found out that the best way to do this still placing nested SMO Create.vi in onCreate.vi of upper level, following that Start.vi -> onStart.vi ; Stop.vi -> onStopped.vi; Destroy.vi -> onDestroyed.vi.

Do you think it would be painful to adapt my current project to the framework example commited 2 days ago, and would be good for a final user software? As most of the logic is inside Process.vi, I imagine it to be simple as changing heritance and copying some Case Structure States..

francois-normandin commented 8 years ago

It would take you some refactor to use the Launch Dependencies method introduced lately, but I wouldn't qualify that as "painful". It depends on how much of a crunch you are in to deliver your product. In general, if it ain't broken, don't fix it ;-).

About your other comment on using onCreate and onDestroyed... your application may still work because you seem to use dependencies as byValue in your class private data, but in general I would recommend that you consider "onCreated" as the time when you are absolutely certain that all references are valid in your object. Same thing applies for onDestroyed where you have to consider that all references have already been destroyed. I think this would be a very good documentation for me to produce, which ties into your other comment in a different thread.

In theory, SMO Framework is designed to be used as both byValue and byReference at the same time. It gives a lot of flexibility. but it also means that we need to keep in mind the lifetime of the object when we use the different activity overrides (onCreate, onCreated, ...). In a byValue design, it doesn't matter which one of onCreate or onCreated you use as the private data will be in the same state when you exit the Create static method, however any use of the framework's API such as "Get Public Events" might cause you some headaches if you don't let the framework create and destroy its self references. I would also advise to use onDestroyed only for things such as dumping a log file when all is done, as long as you use it by Value for accessing the private data.

3esmit commented 8 years ago

I had that problems with byRef Template, and did how you suggested. When I got 3 levels of nested It always got running loops that needed me to close project and reopen. Isolated dependency of 2 level worked well, but when aggregated to another top level it hanged. Also some specific SMOs with basic code or no process.vi that worked as expected in Test.vi was not stopping, having me to close project to force abort of all vis. As I was in need of developing HAL for a project, I looked up how HAL-Webinar works and started using the enumerateDependency.

I found some issues about that.. When I suply a name for it, i.e Endurance.DB it turns out EnduranceDB without the separating dot. image This happens because of this: image To fix I need to put no name and change the pattern suggested in SMO.lvclass:enumerateDependencies.vi image

francois-normandin commented 8 years ago

Thanks for the report about deeply nested dependencies. Hanging processes are usually a result of mishandling an error. This feature is new and I will investigate as well what are the reasons for this hang-up.

As for the Dependency name... The general convention for OOP is to use dot notation to define a more specific child implementation. Allowing to use a dot in the name of the dependency causes all kinds of issues down the line and as you've found, the unique name for a subsystem is assigned by filtering some characters. Because the class name is used to generate a subsystem name in absence of a user-defined name, the dot is removed. The expectation is that we can use subsystem names as a means to inspect or connect to the SMOs and as such, it needs to follow some standards to allow URL parsing, dynamic dependency injection, interoperability with other languages (Python, .NET, etc.)... This is why both Create and Launch Dependency methods have a unique name output.

francois-normandin commented 8 years ago

I had some time to review the issue this weekend and so far, I am a bit puzzled. I was able top reproduce this lock for nested dependencies with 3 levels or more. VIs stay reserved in the context they were running, but no VIs are still running, as evidenced by the fact that closing the project does not provide a dialog for aborting running processes.

I verified that all the objects are created, started, stopped and destroyed in the right order, and I do not see any callers leaving memory before their dependencies are destroyed. I do not see any leaked references in DETT either, suggesting that all is being handled as it should.

One hint I found is that launching dependencies dynamically does not lock the classes, whereas launching them statically does. The difference in the object's lifetime is that static dependencies are created and destroyed in onStart() and onStop(), whereas dynamic ones are created once the process is running and are also destroyed during onStop(). That might be the key to this enigma. However, there is nothing fundamentally different when using LaunchDependency.vi (dynamic) and onStart.vi (static).

3esmit commented 8 years ago

I found out that this case sometimes throws an error in LabVIEW 'eventoracle.cpp', so my guess is that is a problem with event system, I tweeted NI about this at (https://twitter.com/3esmit/status/753764937105829888) with the following screenshot cnxpmvvweaemab9 jpg-large

The internal error report is the following cf081677-7946-44e7-8d24-8668f12fa282.zip

3esmit commented 8 years ago

Another LVInternalReport of the error in the same condition (JKI classes locked). This error happened at closing project as I remembers... 22d3b12b-9992-4e0c-8aa0-055bd1b8bdb4.zip

I don't know if the problem is just 'locked classes at shutdown', but seemed something more (i was trying to make Endurance.UI.Config.General.lvclass being loaded by Endurance.UI.Config.lvclass that in Endurance.UI.Config.lvclass:Process.vi had a subpanel that loaded Endurance.UI.Config.General.lvclass:Process.vi. The same way Endurance.UI.Config.lvclass loaded by Endurance.UI.lvclass and Endurance.UI.lvclass:Process.vi had a subpanel that laods Endurance.UI.Config.lvclass:Process.vi. Could be some problem in my code, but I was unable to make it work. But using new enumerateStaticDependencies.vi things seemed to work properly, but sometimes it still locks (but now works).

So should I avoid deeply nested dependencies?

francois-normandin commented 8 years ago

I've seen my fair share of .cpp crashes in my career, but eventoracle.cpp is a first, Could it have anything to do with your use of databases? I've mostly seen image.cpp crashes in 2013 due to comment arrows.

As for the dependencies, I tried my test project in LabVIEW 2015 this morning, and it does not lock the classes. I don't have 2014 installed on my home computer, so I can't test that version at the moment. It seems to be an issue that has been resolved since 2013. Check out the "hotfix/nested-dependencies" branch, you will find "JKI SMO Nested Dependencies Example.lvproj" under the /tests folder.

If you are stuck in 2013, I would avoid this feature of the framework for now. I will give this more mileage in 2015 and see how it goes in the coming weeks. As mentioned earlier, DETT shows me that everything is happening in the right order, so if the result is different in 2015 than 2013, it tells me that there was a fix at the language level.

3esmit commented 8 years ago

Switched to your branch and...

First try at 2015:

test_StartSMO (SMO_TestCase)
 ... 
ok 0.030 sec
test_GetPublicEvents_SMO (SMO_TestCase)
 ... 
ok 0.002 sec
test_MultipleStartStopSMO (SMO_TestCase)
 ... 
FAIL 100.048 sec
test_StopSMO (SMO_TestCase)
 ... 
ok 0.020 sec
test_ShowAndHideUI (SMO.UI_TestCase)
 ... 
ok 0.061 sec
test_ShowUI (SMO.UI_TestCase)
 ... 
ok 0.068 sec

======================================================================
FAIL: test_MultipleStartStopSMO (SMO_TestCase)
----------------------------------------------------------------------

----------------------------------------------------------------------
Ran 6 tests in 100.509s

FAILED (failures=1)

image

image

Try to retest buyt VI tester got somehow stuck so I closed and retested... Second run (took a lot of time but passed)

test_StartSMO (SMO_TestCase)
 ... 
ok 0.023 sec
test_GetPublicEvents_SMO (SMO_TestCase)
 ... 
ok 0.004 sec
test_MultipleStartStopSMO (SMO_TestCase)
 ... 
ok 0.165 sec
test_StopSMO (SMO_TestCase)
 ... 
ok 100.024 sec
test_ShowAndHideUI (SMO.UI_TestCase)
 ... 
ok 0.049 sec
test_ShowUI (SMO.UI_TestCase)
 ... 
ok 0.044 sec

----------------------------------------------------------------------
Ran 6 tests in 100.677s

OK

Now retested 3 time (without VI Tester restart)

test_StartSMO (SMO_TestCase)
 ... 
ok 0.021 sec
test_GetPublicEvents_SMO (SMO_TestCase)
 ... 
ok 0.004 sec
test_MultipleStartStopSMO (SMO_TestCase)
 ... 
ok 0.162 sec
test_StopSMO (SMO_TestCase)
 ... 
ok 0.020 sec
test_ShowAndHideUI (SMO.UI_TestCase)
 ... 
ok 0.042 sec
test_ShowUI (SMO.UI_TestCase)
 ... 
ok 0.044 sec

----------------------------------------------------------------------
Ran 6 tests in 0.579s

OK

Tryed a lot of times (without VI tester restart) and then got stuck again in Multiple StartStop (and if select it in list sutck all windows until 100 sec reached)

test_StartSMO (SMO_TestCase)
 ... 
ok 0.021 sec
test_GetPublicEvents_SMO (SMO_TestCase)
 ... 
ok 0.004 sec
test_MultipleStartStopSMO (SMO_TestCase)
 ... 
FAIL 100.062 sec
test_StopSMO (SMO_TestCase)
 ... 
ok 0.019 sec
test_ShowAndHideUI (SMO.UI_TestCase)
 ... 
ok 0.042 sec
test_ShowUI (SMO.UI_TestCase)
 ... 
ok 0.041 sec

======================================================================
FAIL: test_MultipleStartStopSMO (SMO_TestCase)
----------------------------------------------------------------------

----------------------------------------------------------------------
Ran 6 tests in 100.479s

FAILED (failures=1)

Retryed and again same problem..

I will try now in 2013

3esmit commented 8 years ago

I missed the button to "close and comment". I'm checking if I'm really on your branch.. (im using that gitwindows)

3esmit commented 8 years ago

Well, that errors are intersting. Also 2013 run better then 2015. But now I see you created a specific lvproj for Nested Dependencies, sorry for that mistake, anyway, why the tests fails sometimes?

The 2015 runs fine, start lock classes, stop unlocks them. 2013 VIs got locked after start and stop, forcing reopen project to edit files.

francois-normandin commented 8 years ago

I have not created these tests, I will have to take a look at them. I know I should have ;-) The tests might not be adapted to the asynchronous nature of the dependencies or there might just be some timing out.

3esmit commented 8 years ago

about eventoracle.cpp I can't say. I searched for it and some people in 2009 complaining about it using ActiveX. Im using .NET, so it's kind of same family, so maybe it could be.

About getting stuck, this is really annoying for working.. I'm having that with simple nested in new enumerateStaticDependencies method.

francois-normandin commented 8 years ago

I looked at the Unit Tests and found two race conditions when starting and stopping SMOs in a tight loop. Those race conditions required the addition of onStopped( ) activity to properly address the first condition. I took the opportunity to complete the symmetry with a onStarted( ) activity. turned out that the second condition was the result of an incorrect order of generating the onStarted( ) event, so encapsulating the call was key to quickly solving the second race condition.

onStopped event has been moved to occur after the asynchronously launched process to return its last error before being disposed. It was previously assumed that onStopped was equivalent to onProcessStop... It might have been true in the first version of the SMO class, but that is no longer the case. image

Package 1.1.5.29 has all the fixes. Unfortunately, it did not solve the class locking issue in 2013, so it is unrelated.

I think we'll plan to blog about the activity diagram for the SMOs. That would probably be a useful topic.

3esmit commented 8 years ago

I think my artistic code at 8/#issuecomment-232218351 is a good example of bad pratice in dealing with multiple nested dependencies. That is really hard to mantain, now with enumareteStaticDependencies its simple. BTW, I stopped using that much larger line as it confuses me with arrays.

francois-normandin commented 8 years ago

Fixed the issue of class locking after everything stops correctly... See this great blog article from Robert Smith: https://labviewcoder.com/2016/07/06/quick-tip-asynchronously-launching-vis-the-right-way/

StartProcesses.vi used this pattern. I changed for non-strictly typed VI and the lock is gone in LV2013.

image