Jump to content

Are *you* misusing the Not A Refnum function and putting your app at risk?


Recommended Posts

You're right... I need to flip the combined case to the Error case instead of No Error. I took the screenshot before fixing that.

Corrected image (I'll ask Moderator to move this image into the original post)

post-5877-0-60034300-1331572572.png

Link to post
Share on other sites

I was curious about the performance difference between the left implementation in the first image and the left implementation in the second image, so I whipped up a quick benchmarking test. (Unfortunately I'm still working through some premium membership issues so I can't upload any files or images.) I created an array of 250,000 queues and iterated through them once using Not A Refnum, then again comparing them against a queue constant using the Is Equal function.

Not A Refnum took 39 ms while Is Equal took ~1 ms. So yeah, Not A Refnum definitely takes significantly longer. But Not A Refnum still only takes ~150 nanoseconds per call, so is it worth worrying about? Perhaps if you're running in a really tight loop, but I have to admit I'm rarely concerned about a time hit that small.

(As a side note, creating all those queues took 53.1 seconds for an average of 212 microseconds each. That's 1,000 times longer than the Not A Refnum function.)

--------------------------------

To phrase AQ's point in a slightly different way, it's a question of pretesting versus posttesting. In general, pretesting feels cleaner to me. I find it easier to reason through the code when fewer errors are possible. However, operations on a reference can't be pretested without exposing your self to a race condition. The only thing you can do is attempt the operation and then see if it worked as AQ shows in the "Combined Good Usage" example.

Using the Not A Refnum function to decide to create a refnum is ok because the zero refnum cannot be destroyed/released in a parallel thread.

I don't think it is a practice that should be encouraged. In fact, it should probably be actively discouraged. You're still exposing yourself to a race condition. Let's use your "Performance Problem" snippet as an example and ignore the performance issues you raised.

The purpose of that snippet is to guarantee the output terminal contains a valid refnum. You are correct that we get the desired behavior in those cases where the input terminal has a zero refnum. It will fail the test, allocate a new queue, and because nobody else has that new refnum it is guaranteed to be valid.

But if the input terminal already has a valid refnum on it all bets are off. You end up with the exact same race condition as in your "Bad Usage" example. The only place we can safely use that snippet is in situations where we know the input queue will have a zero refnum, and if we know that there's no reason to check the refnum in the first place.

What is an acceptable use case for the Not A Refnum primitive?

Rather than trying to list the specific examples of where it is and isn't okay to use Not A Refnum, I'd just go with,

Pretesting a reference before performing an operation on it creates race conditions.

It doesn't matter what function is used for the pretest (Not A Refnum, Is Equal, etc.) or what you're actually testing for (valid refnum, specific data values, etc.,) if you're pretesting to decide execution flow there is a race condition. (Unless, as you pointed out, there is nothing happening in parallel.)

Would the implementation of this idea make this situation less likely?

No, but it would make writing code with race conditions easier and more visually pleasing, possibly increasing the number of users who encounter that race condition.

Link to post
Share on other sites
Cross link from the Idea Exchange: http://forums.ni.com...r/idi-p/1022833 Would the implementation of this idea make this situation less likely?

I'll copy my response from the LV IdEx over here:

I have a tendency to agree that this idea could promote "seemingly safe" but actually bad practices, and after AQ's warning in 2009, I actually came to realize this is probably not a good feature to add to the language. Granted, it would be acceptable and syntactically slick to execute code in the <Not a Refnum> case, but the tendency for developers to place code in the <Valid Refnum> case makes it dangerous (even my snippet to promote this Idea makes this mistake!).

I respectfully suggest this Idea be Rejected on the grounds of potentially causing more problems than the syntactical sugar is worth (and also because it complicates the case selection of class types, a much cooler and more useful Idea)

Link to post
Share on other sites

Cross link from the Idea Exchange:

http://forums.ni.com...r/idi-p/1022833

Would the implementation of this idea make this situation less likely?

Nope. It would make it just as likely... the structure node could not hold the lock on the refnum because you're using nodes that need to be able to lock the refnum inside the structure node. Even if you taught the nodes to recognize when they are directly within a structure node AND the refnum they're given is the one that the structure node is locking, it wouldn't help if the nodes were in a subVI inside that structure node.

Ultimately, this is the problem with references and is why I push so hard against being able to wire a reference directly to a by value terminal for method invocation. Without a single function that checks "is valid and if so do the operation" atomically, there's a race condition. If I write a by value class that has methods "Is Initialized" and "Do Something", wiring a reference to those two functions in sequence is incorrect usage. Whoever writes the reference API needs to build a single VI that locks the reference once and then does both of those operations before unlocking. These sorts of race conditions become ubiquitous very quickly and they're nearly impossible to debug. Heck... half the time I can't even convince people they have a bug because "it works just fine when I test it." And it will... until you've deployed it on your largest customer's end system. And then it will mysteriously fail.

Not A Refnum took 39 ms while Is Equal took ~1 ms. So yeah, Not A Refnum definitely takes significantly longer. But Not A Refnum still only takes ~150 nanoseconds per call, so is it worth worrying about? Perhaps if you're running in a really tight loop, but I have to admit I'm rarely concerned about a time hit that small.
Your time will vary depending upon the type of the refnum. Some types of refnums take longer to validate than others, depending on what system is used for the backing store for that refnum type and, in some cases, how many refnums are in memory. Also, some plug-in type refnums may require calling into a DLL using the UI Thread, so there could be a thread swap. Finally, the "equals not a refnum" test is deterministic, whereas the other test is not. Edited by Aristos Queue
Link to post
Share on other sites

Well .this isn't a new phenomena. And the accepted way is to create a LV2 style global to get the reference. In fact. I supplied exactly that solution for this thread (although it was for events). I've also noted that JGCode uses this method in his classes. KISS.

Link to post
Share on other sites

And the accepted way is to create a LV2 style global to get the reference... KISS.

Just because it's an accepted idea doesn't mean it's a good one. I took the liberty of downloading the code you provided for the user on the other thread and poked around. Before I comment on the code, let me first say this...

--Your solution is based on the code supplied by the other user, who was trying to address a very specific problem. Sometimes we help people bandage the cut on their arm without pointing out the railroad spike buried in their skull. I understand that. In that particular case it sounds like the solution you gave him does the job. I also understand examples are necessarily simplistic and cannot cover all the cases we are likely to encounter. Still, I'm not sure it's an example of a good general purpose solution.

First, the purpose of your Not A Refnum test is simply to initialize the references on startup. If anyone calls the AE's Close action the entire thing breaks down. The NAR test isn't particularly helpful for keeping the system up and running. The limitations of that AE would be far more clearly communicated by replacing Not A Refnum with an Is First Call function. (Or even better, an explicit "Allocate References" action.)

Second, using an AE as a poor man's mutex can't prevent a race conditions as long as the refnum is exposed to other code. The only way to verify a race condition does not exist is by inspecting the code and making sure no operations are performed on that reference anywhere other than in the AE. Imagine how much fun that will be on a large project.

There are other things that smell too (one event refnum attached to multiple event structures(!?), no clear owner of the references, etc.) but they are sidebars to the question of pretesting references.

Link to post
Share on other sites

ShaunR: The AE solution doesn't provide any protection against the race condition I originally posted about as long as the refnum is shared anywhere else... you'd have to encapsulate all operations on the refnum within the AE.

I'm a bit confused on AQ's second "Good usage"... why throw away the error, rather than just handling it downstream?

Because, in general, the whole point of "check if the refnum is valid and only do the operation if it is valid" is from use cases where you don't want an error if the refnum is invalid. So doing the op but ignoring the error gets you that effect.
Link to post
Share on other sites

Because, in general, the whole point of "check if the refnum is valid and only do the operation if it is valid" is from use cases where you don't want an error if the refnum is invalid. So doing the op but ignoring the error gets you that effect.

I was curious about this too. I think it's going to vary from developer to developer because personally I'd want some feedback if the enqueue failed. One of the modules we use internally has two error outputs on its action VIs: one for the outcome of the action of the VI and one for the pass-through and/or generic logic of the VI. Sometimes the terminals get merged, sometimes they are handled separately, sometimes the action error just gets discarded, but it's nice to have that flexibility.

In short, I definitely agree with "the whole point", but think it's valuable to expose the error output anyway.

Link to post
Share on other sites

Asbo: You miss the point... these are cases where you *don't care* whether the enqueue succeeds or not. Things such as "I am a task that fires this event if there's someone listening... if there's no one listening anymore, I keep doing my work." Literally, you don't want *any* notification if the thing fails. That's almost always the reason that I see "Not A Refnum" included in people's code.

Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.