Skip to content

fix: Resolve sequencer last_start regression and gateway proxy concurrency crash#1336

Open
khyatimahendru wants to merge 3 commits into
faucetsdn:masterfrom
khyatimahendru:seq-fix
Open

fix: Resolve sequencer last_start regression and gateway proxy concurrency crash#1336
khyatimahendru wants to merge 3 commits into
faucetsdn:masterfrom
khyatimahendru:seq-fix

Conversation

@khyatimahendru
Copy link
Copy Markdown
Collaborator

@khyatimahendru khyatimahendru commented May 21, 2026

  • Generalize sequencer last_start regression guard in SequenceBase.java to prevent backwards timestamps updates on out-of-order delivery.
  • Use ConcurrentHashMap to shield against ConcurrentModificationException during Jackson serialization.
  • Patched CatchingScheduledThreadPoolExecutor to unpack and log exceptions thrown by Scheduled Futures.

…rency crash

- Generalize sequencer last_start regression guard in SequenceBase.java to prevent backwards timestamps updates on out-of-order delivery.
- Introduce ThreadSafeHashMap extending HashMap inside udmi.util to shield against ConcurrentModificationException during Jackson serialization.
- Patched CatchingScheduledThreadPoolExecutor to unpack and log exceptions thrown by Scheduled Futures.
- Integrated ThreadSafeHashMap inside PointsetManager.java to instantiate clean thread-safe state point maps.
@khyatimahendru khyatimahendru requested a review from grafnu May 21, 2026 08:02
@grafnu
Copy link
Copy Markdown
Collaborator

grafnu commented May 21, 2026

Is there a better way to implement the ThreadSafeHashMap using a standard library class instead?

Yes, there absolutely is! In fact, implementing a thread-safe map by extending HashMap and synchronizing every method is a classic "reinventing the wheel" anti-pattern in modern Java.

While your current implementation solves the ConcurrentModificationException issue by returning snapshot copies in entrySet(), keySet(), and values(), it suffers from major performance bottlenecks because it synchronizes entire read and write operations on a single lock.

Copy link
Copy Markdown
Collaborator

@grafnu grafnu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for a complete new class!

@khyatimahendru
Copy link
Copy Markdown
Collaborator Author

@grafnu I originally intended to use the standard ConcurrentHashMap but in the auto-generated classes PointsetState and PointsetEvents, the points collection is declared strictly as a concrete HashMap rather than the Map interface:

    @JsonProperty("points")
    @JsonPropertyDescription("Collection of point names, defining the representative point set for this device.")
     public HashMap<String, PointPointsetState> points;

    @JsonProperty("points")
    @JsonPropertyDescription("Collection of point names, defining the representative point set for this device.")
    public HashMap<String, PointPointsetEvents> points;

Since ConcurrentHashMap is a subclass of Map and not HashMap, I had to add a new class.

@khyatimahendru khyatimahendru requested a review from grafnu May 21, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants