[JSR223][Jython]What is the correct way to avoid a race condition getting an item value inside a /received update/ triggered rule?

[rlkoshak (Rich Koshak)]

I think this might be a case where understanding how OH works under the covers might explain the behavior. This might not be something that has been seen or reported before as perhaps other things cause Rule triggers to slow down enough that there isn’t such a race condition. Note: I’m taking your word for it that there is a race condition here. I haven’t looked at the posted code that closely to confirm.

I’m hoping that @5iver, our foremost Jython and JSR223 expert will chime in. If this is indeed a problem with Jython, as the evidence seems to indicate (whether or not it’s Jython’s fault) we will have more people run into this problem. Also, rossko57 and I might be off base and their might be some detail we don’t know about that would make this behavior.

OK, now for a deepish dive into OH. OH is driven by an EventBus. Events like updates, commands, and changes get published to this event bus. All other parts of OH operate off of this bus. That means that when an update gets published to th

e bus every part of the system that cares about that event pick it up and start processing it in parallel. This means that the Rule get’s triggered at the same time that the Item Registry (I’m guessing which part actually changes the Item) picks up the event to update the Item which happens at the same time that Persistence picks up the event to store it into the database, and so on.

Given this is all occurring at the same time, strictly speaking you should never rely on anything else to have finished before the Rule get’s triggered. So, as you have found, if you want guaranteed to get the state/command that triggered the Rule then you need to get that from the event, not from the Items. NOTE: changed is a little different because it is the Item registry that generates the event so you know the Item has entered the new state before the Rule triggers. But even here there is no guarantee that the Item hasn’t changed state again before your Rule triggers.

In practice, the timing usually works out that you can rely on the Item’s state for update and even command triggered Rules. But not all the time, especially for command triggered Rules. But every now and then, especially on fast machines, you discover cases where the timing breaks down.

NOTE: I’m basing the above on years of experience using OH, helping users on the forum, and experience I’ve had coding similar event based systems. I have not looked at the OH code so I may be wrong.

But if I am correct on this, it means that it is a fundamental limitation of the architecture and not something that can easily be fixed. It probably isn’t a problem with Rules DSL because Rules DSL is s-l-o-w. It takes long enough for the Rule to kick off that the Item registry has already processed the update as necessary.

@leif

…is supposed to be the exact equivalent of
Rules DSL:

It is the functional equivalent. There is no such thing as an exact equivalent when you are dealing with to such drastically different programming languages with vastly different structure, initialization, etc. In this case, when the Rules DSL Rule runs, the ir.getItem("RaceItemSwitch") part is executed before the Rule triggers (if I understand correctly).

@leif

Adding a short delay statement before reading the item statement band-aids the problem (it behaves properly) but at the expense of performance.

There is no solution to this problem that won’t be at the expense of performance. But in a home automation context, so what? If 100 msec or so makes enough of a difference to become a problem then OH isn’t the tool for you and you should be using a real time system (or moving the operation to the device itself).

[leif (Leif Claesson)]

Thanks for all your tutorials and design patterns, @rlkoshak! Pleased to make your acquaintance.

Interesting. It could indeed be that Rules DSL is slow enough that it’s a not an issue.

Interesting about the EventBus too, that makes sense. Although, there are a couple of ways to solve the issue.

One way, which would be a pretty big change, would be if the eventbus listeners had priorities, so that the ItemRegistry got the update synchronously, first, and then every other listener got it. Without knowing anything about the inner workings of OH, I’m rather audacious to even suggest it. It is a viable and pretty obvious programming model (i’ve done similar things myself in event-based systems) – it could even be that OH already has it and the JSR223 binding forgot to flag itself as lower priority than the item registry? Wishful thinking with no factual basis.

Here’s a possible safe way to solve the problem in the JSR223 layer, though.
What if “ir” when referenced within a rule got you a subclassed version of the itemregistry, where getItem() instead returns a subclassed version of the item, which in turn returns event.itemState if available in the current context, but otherwise passes the request through to the real item registry like it normally does?

@rlkoshak

It is the functional equivalent. There is no such thing as an exact equivalent when you are dealing with to such drastically different programming languages with vastly different structure, initialization, etc.

You are 100% correct, please forgive my hyperbole! I meant functional.

Indeed a 100 millisecond delay would be inconsequential. But, couldn’t it be in the back-end so that it “just works” without one having to worry about it when writing scripts?

I like the subclassing idea better though, that would probably be transparent, and appear to “just work”.

My self-proclaimed atrocity above wouldn’t be that if it were in the core code… then it just becomes business logic. It’s when you have to work around low-level issues like synchronization in high-level scripting that things get ugly, in my humble opinion.

[ricadelic (Rick Jongbloed)]

I’ve also seen race conditions with Jython. Especially it seems the item registry is loaded once the rule is run. When updating an item, it seems the state of the item isn’t updated until the script is released.

I’m always using the items.item_name wait to find out the state of the item, and finding out which item triggered the rule by checking the events object. I only use ir.getItem(“MyButton”) when needed. you should be able to use items.mybutton which returns the state of the item.

To make sure unexpected things don’t happen, i usually create an initialize part of a script that checks the state of all the relevant items. If items end in unexpected states, a rule is not executed, but rescheduled by a timer.

I do agree we should be able to fix this in the JSR223 Jython helper code.

[rossko57]

@ricadelic

When updating an item, it seems the state of the item isn’t updated until the script is released.

It may well not compare, but in DSL
someItem.postUpdate(newState)
does not, and is not intended to, have immediate effect. It puts the update request onto openHABs bus is all.
The actual update occurs asynchronously. It is not a variable assignment, it is a request for action.
For example, this will almost always “fail”
someItem.postUpdate(newState)
logInfo("test", "Bet you this is the old state " + someItem.state.toString)

It’s not a bug and it’s not a programming handicap, the rule does after all know what the shortly expected newState is.
Certainly confuses authors until they grasp the event bus idea.

[mjcumming (Michael Cumming)]

Rick have you opened an issue on github?

[leif (Leif Claesson)]

@rossko57

someItem.postUpdate(newState)
does not, and is not intended to, have immediate effect.

That is the same in Jython.
It’s also the same in windows application (WIN32 API) programming. SendMessage() waits for a response, PostMessage() returns immediately. Makes perfect sense to me.

[leif (Leif Claesson)]

@ricadelic

I’m always using the items.item_name wait to find out the state of the item

THIS intrigued me – could you give me an example?

[CrazyIvan359 (Michael Murton)]

@leif the race condition you are experiencing is not a bug, it is a result of the parallel nature of openHAB, as @rlkoshak mentioned. The reason you are unable to duplicate it is because, as you mentioned, the DSL rules are slower to run than JSR223 rules, combined with your platform. My openHAB is running on an old desktop and I could duplicate your race example in the DSL if I were to set it up.

The solution, from experience, is to use event.itemState to get the new state the item will have instead of getting it from the registry. You should also be using the equivalent in the DSL if you need to make rules that trigger on received update to prevent race conditions. I had the same confusing race conditions when I was first starting out with openHAB and the DSL.

You may want to check out the new documentation for the Helper Libraries. We just finished the majority of it, finishing touches to come. You should look at the Event Object Attributes page for details about what values you get for each of the trigger types.

Feel free to reach out to me if you have any other issues. Rich, feel free to tag me as well as Scott if you need an assist on JSR223 stuff.

[leif (Leif Claesson)]

Thanks, Michael. Are you saying this is the correct solution to acting on multiple items in one rule?

@leif

	stateMyButton=event.itemState if event.itemName=="MyButton" else ir.getItem("MyButton").getState()
	stateTestSwitch=event.itemState if event.itemName=="Test_Switch_1" else ir.getItem("Test_Switch_1").getState()

If that has to be done in every rule that acts on multiple items (not an uncommon thing), wouldn’t it be better if it was in the back-end code so we wouldn’t have to script it every time?

I do understand the “it has always worked this way”, but more and more people use openHAB every day, and it’s always evolving, so I’m asking, is there a way to make this better? Would there be any drawbacks to eliminating this race condition and making the behavior consistent?
It’s hard to imagine that there could be any script relying on the race condition to function properly… short of using it is a 1-bit random number generator?

As a product developer, I’m often on the other side of this equation. Often, “by design” is the right answer. But, there are times when I’m explaining a tricky concept to a customer (or the support department) when I realize that it would be easier and save everybody a lot of future grief if I just made it better, rather than explain the complicated current procedure. I’m wondering if this is one of those times.

New documentation looks great by the way! Bookmarked, will read.

[CrazyIvan359 (Michael Murton)]

@leif

If that has to be done in every rule that acts on multiple items (not an uncommon thing), wouldn’t it be better if it was in the back-end code so we wouldn’t have to script it every time?

In your example you need it because you have multiple items able to trigger the rule and you are using the received update trigger, that is far less common. I have some rules that I want to fire every time the device provides a status update, so that I can make sure it is set to the value the automation wants it to be at. I don’t think your rule needs to use received update, I think changed would be enough. changed fires after the item state has been updated, so you don’t have a race between your rule and the event update code in openHAB.

@leif

I do understand the “it has always worked this way”

That is not what this is, the received update and changed triggers work in different ways, as intended. The received update trigger fires when an item state update event is put on the event bus. The changed trigger fires after the aforementioned update event is processed and the update changed the state. Often changed is the more appropriate trigger to use. This is not a case of “by design” it works this way so that’s it, this is “by design” both triggers are available because they are different and each have their use case.

In hopes of helping you along your openHAB/Python learning curve, I took your script in the first post and rewrote it. I have switch over to the easier way to do logging, sending commands, and getting item states. I have also demonstrated a more readable way to get the most relevant item state. And I have also changed some of your conditionals to be more Pythonic. Contrary to my above suggestion that changed might be a more appropriate trigger, I have left it as received update because I don’t know what your exact use case is. Also to demonstrate in case you ever need to use received update, because there may be cases where you need the rule to run even if the item state did not changed, at which time it’s up to you to get the most relevant item state as required by your code.

from core.rules import rule
from core.triggers import when

"""
from org.slf4j import LoggerFactory
def logprint(text):
    pass
    LoggerFactory.getLogger("org.eclipse.smarthome.automation.examples").info(text)
"""
from core.log import logging, LOG_PREFIX
log = logging.getLogger("{}.nightlights".format(LOG_PREFIX))

"""
@rule("Sunset Rule", description="", tags=["schedule"])
@when("Channel \"astro:sun:local:civilDusk#event\" triggered START")
def Sunset_Decorators(event):
    item=ir.getItem("DarkOutside")
    item.send(ON)
Use events.sendCommmand("string", "string")
"""
@rule("Sunset Rule", tags=["schedule"])
@when("Channel astro:sun:local:civilDusk#event triggered START")
def Sunset_Decorators(event):
    events.sendCommand("DarkOutside", "ON")

@rule("Sunrise Rule", description="", tags=["schedule"])
@when("Channel \"astro:sun:local:civilDawn#event\" triggered START")
def Sunrise_Decorators(event):
    events.sendCommand("DarkOutside", "OFF")

@rule("Night Lights", tags=["schedule"])
@when("Item DarkOutside received update")
@when("Item SleepMode received update")
def NightLights_Decorators(event):
    """
    isDarkOutside=1 if itemDarkOutside.getState() == ON else 0
    isSleepMode=1 if itemSleepMode.getState() == ON else 0
    Don't cache the item, use True/False
    """
    isDarkOutside = True if ir.getItem("DarkOutside").getState() == ON else False
    isSleepMode = True if ir.getItem("SleepMode").getState() == ON else False

    if event.itemName == "DarkOutside":
        isDarkOutside = True if event.itemState == ON else False
    else:
        isSleepMode = True if event.itemState == ON else False

    """
    logprint("dark: " + str(isDarkOutside) + "   sleep: " + str(isSleepMode))
    string.format is your friend, you can even use names for the fields to replace
    """
    log.info("dark: {dark}   sleep: {sleep}".format(
        dark=isDarkOutside,
        sleep=isSleepMode
    ))

    # in Python you can 'if var' to check 'if var == True'
    events.sendCommand("NightLights", "ON" if isDarkOutside else "OFF")
    events.sendCommand("MainGateLights_On", "ON" if isDarkOutside and not isSleepMode else "OFF")
    itemMainGateLights=ir.getItem("MainGateLights_On")
    itemMainGateLights.send(ON if isDarkOutside!=0 and isSleepMode==0 else OFF)

[leif (Leif Claesson)]

Oh wow. Thank you! I should have learned this years ago. I can’t believe I used to do everything in C++. When all you have is a hammer, everything looks like a nail! I’m liking Python more by the minute.

@CrazyIvan359

string.format is your friend, you can even use names for the fields to replace

… I can’t say anything else!

Hey, in the rule, instead of first getting the state from the item registry and then overriding (which violates DRY by repeating each item name), what do you think about this:

def GetUpdatedItemState(itemName, event)
	return ir.getItem(itemName).getState() if itemName!=event.itemName else event.itemState

...
    isDarkOutside = True if GetUpdatedItemState("DarkOutside",event) == ON else False
    isSleepMode = True if GetUpdatedItemState("SleepMode",event) == ON else False

Regarding update versus command, it’s exactly because I want to rule to run also when the state is reiterated that I chose “received update”. Z-wave isn’t perfect, and power outages are not unheard of here, so it’s good if things eventually get to the correct state automatically even if the system was off during a trigger. Although I haven’t done it yet, I plan to have the DarkOutside state refreshed every 5 minutes or so… so that it will re-trigger as “Off” throughout the day, and re-trigger as “On” throughout the night. That way any inconsistency will eventually correct itself, no matter where it came from.

I’m planning to do this by having a rule on a 5-minute cron-triggered rule check whether we’re between civilDawn and civilDusk or not, and update DarkOutside accordingly.
How would you recommend doing that in an elegant manner in Python?

Here’s how I had it in lua in Fibaro HC2. Absolutely fugly but worked. Note that there is no way for fibaro lua scripts not to be fugly (no way not to hardcode item IDs for one) so you get desensitized after a while.

--[[ 
%% properties 
%% autostart 
%% globals 
--]] 


--night light maintenance
--kitchen ceiling fan relay on
--(to prevent ceiling fans being off after power outage)


local idKitchenFans = 133;
local idNightLights = 137;


-- Initial parameters ------------------------------------------------------- 
 
if (fibaro:countScenes() > 1) then 
	-- fibaro:debug("SCENE ABORT --------------------------") 
	fibaro:abort() 
end
 

 
-- Functions ----------------------------------------------------------------- 
 
-- function is changing time in text format  "HH:MM" or os.date("*t") 
-- to number of minutes from midnight 
 
function toMinutes(czasHHMM) 
	local a 
	if type(czasHHMM) == "string" then 
		a = tonumber(string.sub(czasHHMM, 1, 2)) * 60 + tonumber(string.sub(czasHHMM, 4, 5)) 
	else    
		a = tonumber(czasHHMM.hour) * 60 + tonumber(czasHHMM.min) 
	end 
	return a 
end 
 
-- changing minutes to "HH:MM" 
 
function toHHMM(minutes) 
	local b = string.format("%02d",((minutes/60*100) - ((minutes/60*100) % 100))/100) 
	local c = string.format("%02d",minutes - (tonumber(b)*60)) 
	local d = b..":"..c 
	return d 
end 
 
-- end of Functions----------------------------------------------------------- 
 
 
function fnNightLights()
  
-- counting how many minutes is from midnight to sunset and sunrise
 
	local sunriseMinutes = toMinutes(fibaro:getValue(1, 'sunriseHour')) 
	local sunsetMinutes = toMinutes(fibaro:getValue(1, 'sunsetHour')) 
 
	-- counting how many minutes is from midnight to now 
	 
	local nowMinutes = toMinutes(os.date("*t")) 
  
	if nowMinutes > sunriseMinutes+5 and nowMinutes < sunsetMinutes-5 then
    
	  fibaro:call( idNightLights, 'turnOff' );
    
    else

      fibaro:call( idNightLights, 'turnOn' );
    
    end

  
	fibaro:call (idKitchenFans, 'turnOn' );
   	

  setTimeout(fnNightLights,5*60*1000);

  
end -- end while


fnNightLights();

```lua


Sorry for subjecting your eyes to it. That's what I had. I'll bet you could do it on a single line in python. Will you accept the challenge? :)

[5iver (Scott Rushworth)]

@CrazyIvan359

log.info(“dark: {dark} sleep: {sleep}”.format( dark=isDarkOutside, sleep=isSleepMode))

Since you’re only using the logger in the rule function, drop the import and use the log attribute added by the rule decorator…

NightLights_Decorators.log.info("dark: {dark}   sleep: {sleep}".format(dark=isDarkOutside, sleep=isSleepMode))

For any rules DSL users seeing this and freaking out about having to use the getState() method, Items also have a state attribute. But instead of getting the Item to get the state, it’s much easier to just use the items object provided in the default script scope (it’s at the bottom of this list). This provides the states of all Items as a dict…

isSleepMode = True if items["SleepMode"] == ON else False

@leif

I’m planning to do this by having a rule on a 5-minute cron-triggered rule check whether we’re between civilDawn and civilDusk or not, and update DarkOutside accordingly.

I’ve only looked briefly at your thread and the rule @CrazyIvan359 put together, but I think you could implement this by just adding a cron trigger to the rule. Also, your rule shrinks quite a bit if you put the logic into a group, and could lose another line if you put the lights in a group too…

Switch DarkOutside "Dark" <moon> (Bench, gLights_Out)
Switch SleepMode "Sleep" <bedroom_blue> (Bench, gLights_Out)
Group:Switch:AND(ON,OFF) gLights_Out "Time for lights out [%s]"

… and then the last rule can be…

@rule("Night Lights", tags=["schedule"])
@when("Item gLights_Out changed")
@when("Time cron 0 0/5 * * * ?")# event will be None
def NightLights_Decorators(event):
    events.sendCommand("NightLights", str(items["gLights_Out"]) if event is None  else event.itemState))
    events.sendCommand("MainGateLights_On", str(items["gLights_Out"] if event is None event.itemState))

The is not much documentation on profiles and I haven’t read throuh the code yet, but if you had the lights in a group with a profile to follow the gLights_Out group, you wouldn’t need a rule at all… but the 5 minute cron trigger throws a wrench into that one!

[CrazyIvan359 (Michael Murton)]

No problem, glad to help.

@5iver

Since you’re only using the logger in the rule function, drop the import and use the log attribute added by the rule decorator…

@5iver

it’s much easier to just use the items object provided in the default script scope

I always forget about these tricks. I need to spend some time updating some of rules to use them. I was not aware of either when I first started writing my JSR223 rules.

@leif

Hey, in the rule, instead of first getting the state from the item registry and then overriding (which violates DRY by repeating each item name), what do you think about this:

What is DRY? I would suggest this modification to prevent errors if event is None:

def GetUpdatedItemState(itemName, event):
    if event is None:
        return items[itemName]
    else
        return items[itemName] if itemName != event.itemName else event.itemState

@5iver 's suggestions for putting your 2 condition items in a group is definitely the way to go. You would be able to use the changed trigger and not have to worry about timing. As for setting DarkOutside, I am doing something similar that boils down to this:

DateTime AstroCivilDawn "Dawn Start [%1$tl:%1$tM %1$tp]" { channel="astro:sun:home:civilDawn#start" }
DateTime AstroCivilDusk "Dusk End [%1$tl:%1$tM %1$tp]" { channel="astro:sun:home:civilDusk#end" }

from core.date import to_java_zoneddatetime, ZonedDateTime

@rule("Update DarkOutside")
@when("System started")
@when("Time cron 0 */5 * * * ?")
def update_darkoutside():
    now = ZonedDateTime.now()
    command = "OFF"
    if now.isBefore(to_java_zoneddatetime(items[AstroCivilDawn])) or now.isAfter(to_java_zoneddatetime(items[AstroCivilDusk])):
        command = "ON"
    events.postUpdate("DarkOutside", command)

[leif (Leif Claesson)]

@5iver

isSleepMode = True if items[“SleepMode”] == ON else False

Thank you, Scott! I will try this. If this takes care of the race condition, then I take everything back.

@CrazyIvan359

What is DRY?

Don’t Repeat Yourself

@CrazyIvan359

if now.isBefore(to_java_zoneddatetime(items[AstroCivilDawn])) or now.isAfter(to_java_zoneddatetime(items[AstroCivilDusk])):

Thank you, this is exactly what I was looking for.

[CrazyIvan359 (Michael Murton)]

The example to set DarkOutside is actually the same principle as the Time of Day example script you can find in the Helper Libraries docs, we just don’t have it working with channels yet.

[matt1 (Skinah)]

@leif

Regarding update versus command, it’s exactly because I want to rule to run also when the state is reiterated that I chose “received update”.

I don’t use jython so it may be different so bear that in mind with the following.

I mostly trigger rules on received command and use the variable called recievedcommand to check what the command was. This way I can move controls without triggering the rule when I wish by using the update method. When I want to trigger the rule I use the command method. I use a lot of Stand alone devices and see openhab as a slave so often I want to update controls because the work has already been done by stand alone devices.

An example is a doorbell where someone presses the push button multiple times. If the button sends an on command, then I will trigger When the item receives the on command so it picks up the multiple pushes with the least delay. Received update I find is the least useful, either use changed or command with the latter working if it is already on and it receives another on command.

I find it useful to watch the event.log file this way you get to see the triggers occurring and in what order they occur.

[leif (Leif Claesson)]

Interesting. I didn’t know about the events log, that could be useful!

I also completely missed the “received command” trigger, hadn’t heard of that one before. So I figured that one could be useful for the Aeotec Wallmote (Z-wave wall controller) since it has a problem with repeated commands. “changed” doesn’t work because it doesn’t change until you press a different button, and “received update” sometimes repeats, sometimes multiple times, sometimes a whole second late… This thing is really not worth the price – a whopping $80. Anyway, unfortunately “received command” doesn’t trigger at all for this device. I’m sure I’ll find a use for it elsewhere, though.

[rubens (Daniel Planets)]

I’m aware this thread is dated but I ran into this issue this week while migrating my .rules to .py
I was about to post my own tests about that possible bug…
Thanks to @leif for document ing the issue and your tests, thanks to @rlkoshak, @mjcumming, @5iver and everyone contributing here for your very helpful explanations and coding advice.
I assume many have run into this or will do so when migrating - is there a link to here in ‘But How do i…’ which I failed to notice? Or , if not, maybe it should…

[5iver (Scott Rushworth)]

@rubens

I ran into this issue this week

What issue are you referring to? There was no issue in the OP, other than not knowing how to properly get the state of an Item, which the helper library documentation definitely covers.

[leif (Leif Claesson)]

I think that is a matter of interpretation – I absolutely still consider it an issue. In my scripts, in the rule handlers I now use event.itemState when reading the triggering item, and ir.getItem to read the state of all other items. And yes, that does work, but as a programmer and designer of other systems I do consider this a workaround. As a script writer it would have been so much easier for me if this had been done in the back-end code, essentially intercepting the return value from getState() with the most current value, if called from within a rule handler with an applicable item.

If the back-end had done that, this whole thread would not exist.

I’m not saying that is the absolutely correct way architecturally… I am however saying that this would remove a scripting pitfall I obviously wasn’t the only one to fall into, hence the resurrection of this thread.