Alarm Tree breaks on duplicate PVs with slashes

[kristjansoln]

The Alarm Tree view allows adding either nodes or PVs as components. When PV names include forward slashes / and duplicates, certain combinations can:

• throw an IllegalStateException in Phoebus,
• make the alarm server appear disconnected, and
• sometimes cause an Unknown alarm tree node error on the alarm server.

How to reproduce

IllegalStateException in Phoebus

1. Start with an empty alarm configuration, open the Alarm Tree view, and create a new node
2. Create new PVs by right-clicking the node, selecting Add component, choosing type PV/s, and entering testpv //testpv in the name box.

Instead of two, this adds just one PV: testpv.

Closing and reopening the Alarm Tree view then throws an exception, and the alarm server appears disconnected:

Exception in thread "JavaFX Application Thread" java.lang.IllegalStateException: Found existing view item for /Accelerator/node1/testpv
	at org.phoebus.applications.alarm.ui.tree.AlarmTreeView.createViewItem(AlarmTreeView.java:308)
	at org.phoebus.applications.alarm.ui.tree.AlarmTreeView.createViewItem(AlarmTreeView.java:312)
	at org.phoebus.applications.alarm.ui.tree.AlarmTreeView.createViewItem(AlarmTreeView.java:312)
	at org.phoebus.applications.alarm.ui.tree.AlarmTreeView.lambda$startup$3(AlarmTreeView.java:214)
	at com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:456)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
	at com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:455)
	at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
	at com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
	at com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$10(GtkApplication.java:263)
	at java.base/java.lang.Thread.run(Thread.java:1583)

Unknown alarm tree node exception in the alarm server

Following the same steps, but entering testpv ///testpv/ in the PV name box, adds three PVs: //testpv/, /testpv/ and testpv.

Attempting to acknowledge the /testpv/ alarm causes an Unknown alarm tree node exception on the alarm server.

 2025-10-30 15:19:47 WARNING [org.phoebus.applications.alarm.server.AlarmServerMain handleCommand] Error for command. path: '/Accelerator/node1/testpv\/', JSON: '{"user":"user","host":"hostname","command":"acknowledge"}'
 java.lang.Exception: Unknown alarm tree node '/Accelerator/node1/testpv\/'
   at org.phoebus.applications.alarm.server.AlarmServerMain.handleCommand(AlarmServerMain.java:352)
   at org.phoebus.applications.alarm.server.ServerModel.checkUpdates(ServerModel.java:237)
   at org.phoebus.applications.alarm.server.ServerModel.run(ServerModel.java:146)
   at java.base/java.lang.Thread.run(Thread.java:1583)

Importing alarm server configuration directly

Identical results occur when importing the configuration directly, e.g.:

java -jar "service-alarm-server-5.0.3-SNAPSHOT.jar" -import $config_file -config "Accelerator"

<?xml version="1.0" encoding="UTF-8"?>
<config name="Accelerator">
  <component name="test">
    <pv name="//testpv">
      <description>desc1</description>
      <enabled>true</enabled>
      <latching>true</latching>
      <annunciating>true</annunciating>
    </pv>
    <pv name="testpv">
      <description>desc2</description>
      <enabled>true</enabled>
      <latching>true</latching>
      <annunciating>true</annunciating>
    </pv>
  </component>
</config>

<?xml version="1.0" encoding="UTF-8"?>
<config name="Accelerator">
  <component name="test">
    <pv name="///testpv/">
      <description>desc1</description>
      <enabled>true</enabled>
      <latching>true</latching>
      <annunciating>true</annunciating>
    </pv>
    <pv name="testpv">
      <description>desc2</description>
      <enabled>true</enabled>
      <latching>true</latching>
      <annunciating>true</annunciating>
    </pv>
  </component>
</config>

As expected, exporting the just-imported configuration with java -jar "service-alarm-server-5.0.3-SNAPSHOT.jar" -export $output_file produces output that does not match the input configuration.

<?xml version="1.0" encoding="UTF-8"?>
<config name="Accelerator">
  <component name="test">
    <pv name="/testpv">
      <description>desc1</description>
      <enabled>true</enabled>
      <latching>true</latching>
      <annunciating>true</annunciating>
    </pv>
    <pv name="testpv">
      <description>desc2</description>
      <enabled>true</enabled>
      <latching>true</latching>
      <annunciating>true</annunciating>
    </pv>
  </component>
</config>

<?xml version="1.0" encoding="UTF-8"?>
<config name="Accelerator">
  <component name="test">
    <pv name="//testpv/">
      <description>desc1</description>
      <enabled>true</enabled>
      <latching>true</latching>
      <annunciating>true</annunciating>
    </pv>
    <pv name="/testpv/">
      <description>/testpv/</description>
      <enabled>true</enabled>
      <latching>true</latching>
      <annunciating>true</annunciating>
    </pv>
    <pv name="testpv">
      <description>desc2</description>
      <enabled>true</enabled>
      <latching>true</latching>
      <annunciating>true</annunciating>
    </pv>
  </component>
</config>

Discussion

I am not exactly sure what breaks, and I'd appreciate some insight. Should alarm configuration inputs be sanitized more aggressively? While the malformed configuration seems to affect only the Alarm Tree UI, it can leave users unable to access or correct the components they created, which can make recovery difficult. Maybe it would be better to reject or normalize PV names at input time to avoid these failures. Can we even reject PV entries with slashes outright, due to them being used to access fields in PVA structures?

This was tested using commit 1a6e90f on the master branch, with Kafka 2.13-4.1.0.

[kasemir]

At this time, PV names with / aren't supported in the alarm GUI because / is used as a path separator in the alarm tree.
When you have configuration "Accelerator", node "Top", PV "X", that's internally handled as alarm tree path /Accelerator/Top/PV.
There is some provision to allow escaped separators like \/ inside a path element, see https://github.com/ControlSystemStudio/phoebus/blob/master/app/alarm/model/src/main/java/org/phoebus/applications/alarm/model/AlarmTreePath.java, but that's obviously not fully utilized by the GUI.

[kasemir]

While at it, you could import the https://github.com/ControlSystemStudio/cs-studio/blob/master/applications/alarm/alarm-plugins/org.csstudio.alarm.beast.test/src/org/csstudio/alarm/beast/AlarmTreePathUnitTest.java from the original implementation into the phoebus sources to make sure that updates don't break the intended behavior.

[kasemir]

Looks like sim://sine works fine, and duplicate PVs should indeed be rejected. We don't need more than one alarm for the exact same PV. The issue you describe might boil down to ////PV internally being reduced to PV in some places, so the duplication isn't consistently detected.

[DKastelic]

I've been looking into this and it seems the issue comes from how AlarmTreePath.makePath(String path, String item) handles leading slashes. Right now, a single leading slash on the item is intentionally ignored, while additional slashes are escaped. Because of that, adding a PV like ///test/ ends up stripping one slash on the client and another on the server, producing inconsistent results.

The fix is straightforward (remove the special case and escape all leading slashes), but that causes two of the above mentioned unit tests to fail:

assertThat(AlarmTreePath.makePath(null, "/root"), equalTo("/root"));
assertThat(AlarmTreePath.makePath("/", "/root"), equalTo("/root"));

With the fix, both would return "/\/root" (with an escaped slash) instead. Since these tests likely reflect expected behavior elsewhere in the codebase, I'm hesitant to remove the special-case logic without feedback.

As I see it, the options are:

1. Remove the special case and update the unit tests.
2. Forbid leading slashes on PVs (with an error message/popup).
3. Automatically sanitize leading slashes from PV names.

Option 1 risks breaking existing code, options 2 and 3 introduce new restrictions on PV names.

I'd appreciate input on which path is preferred.

[kasemir]

My immediate leaning would be to forbid leading slashes on PVs.

I've not seen a PV that starts with a slash in the last 35 years of using EPICS with Channel Access.

In CS-Studio, we support simulated or local PVs that look like sim://sine, so slashes within the name must be supported.

We also somewhat support pvs://some_structure/element for PV Access. Not claiming that this should be a universally supported way to access structure elements within PVA structures; pvget, pvxget etc. may use a different syntax. Not assuming that this will subscribe to "some_structure" with field(element) in the request, or fetch the whole structure. Only suggesting that PV names other than Channel Access may use '/' within a name.

As for how to react to PV names with slashes, AlarmTreePath.makePath should probably be updated to throw an exception, and then all the code that calls it needs to handle the exception. The config dialog in the alarm tree and table GUI could show the exception. Most other code will need to log and somehow move an.