Skip to content

[LLDB][SBProgress] Add a finalize method #128966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions lldb/bindings/interface/SBProgressDocstrings.i
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,14 @@ and will always send an initial progress update, updates when
Progress::Increment() is called, and also will make sure that a progress
completed update is reported even if the user doesn't explicitly cause one
to be sent.") lldb::SBProgress;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets add some usage documentation here for both non-determinisitic and deterministic progress class usage in python here. That way people can see this and use the class as intended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remember this being 1:1 with Progress and not being changed so I would think we should add a new doc string to Increment. Is there anything in particular you want to call out? Many of the deterministic/non-deterministic are predominantly relevant to DAP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made an attempt at summarizing this in increment in manner that doesn't expose too much about the externals. Specifically so that it doesn't go out of date instantly.


%feature("docstring",
"Finalize the SBProgress, which will cause a progress end event to be emitted. This
happens automatically when the SBProcess object is destroyed, but can be done explicitly
with Finalize to avoid having to rely on the language semantics for destruction.

Note once finalized, no further increments will be processed.") lldb::SBProgress::Finalize;

%feature("docstring",
"Increment the progress by a given number of units, optionally with a message. Not all progress events are guaraunteed
to be sent, but incrementing to the total will always guarauntee a progress end event being sent.") lldb::SBProcess::Increment;
5 changes: 5 additions & 0 deletions lldb/include/lldb/API/SBProgress.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ class LLDB_API SBProgress {

void Increment(uint64_t amount, const char *description = nullptr);

/// Explicitly finalize an SBProgress, this can be used to terminate a
/// progress on command instead of waiting for a garbage collection or other
/// RAII to destroy the contained progress object.
void Finalize();

protected:
lldb_private::Progress &ref() const;

Expand Down
12 changes: 12 additions & 0 deletions lldb/source/API/SBProgress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,22 @@ SBProgress::~SBProgress() = default;
void SBProgress::Increment(uint64_t amount, const char *description) {
LLDB_INSTRUMENT_VA(amount, description);

if (!m_opaque_up)
return;

std::optional<std::string> description_opt;
if (description && description[0])
description_opt = description;
m_opaque_up->Increment(amount, std::move(description_opt));
}

void SBProgress::Finalize() {
// The lldb_private::Progress object is designed to be RAII and send the end
// progress event when it gets destroyed. So force our contained object to be
// destroyed and send the progress end event. Clearing this object also allows
// all other methods to quickly return without doing any work if they are
// called after this method.
m_opaque_up.reset();
}

lldb_private::Progress &SBProgress::ref() const { return *m_opaque_up; }
59 changes: 30 additions & 29 deletions lldb/test/API/python_api/sbprogress/TestSBProgress.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,6 @@


class SBProgressTestCase(TestBase):
def test_with_external_bit_set(self):
"""Test SBProgress events are listened to when the external bit is set."""

progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg)
listener = lldb.SBListener("Test listener")
broadcaster = self.dbg.GetBroadcaster()
broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress)
event = lldb.SBEvent()

expected_string = "Test progress first increment"
progress.Increment(1, expected_string)
self.assertTrue(listener.PeekAtNextEvent(event))
stream = lldb.SBStream()
event.GetDescription(stream)
self.assertIn(expected_string, stream.GetData())

def test_without_external_bit_set(self):
"""Test SBProgress events are not listened to on the internal progress bit."""

progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg)
listener = lldb.SBListener("Test listener")
broadcaster = self.dbg.GetBroadcaster()
broadcaster.AddListener(listener, lldb.eBroadcastBitProgress)
event = lldb.SBEvent()

expected_string = "Test progress first increment"
progress.Increment(1, expected_string)
self.assertFalse(listener.PeekAtNextEvent(event))

def test_with_external_bit_set(self):
"""Test SBProgress can handle null events."""

Expand Down Expand Up @@ -65,3 +36,33 @@ def test_with_external_bit_set(self):
stream = lldb.SBStream()
event.GetDescription(stream)
self.assertIn("Step 3", stream.GetData())

def test_progress_finalize_non_deterministic_progress(self):
"""Test SBProgress finalize sends the progressEnd event"""

progress = lldb.SBProgress("Test SBProgress", "Test finalize", self.dbg)
listener = lldb.SBListener("Test listener")
broadcaster = self.dbg.GetBroadcaster()
broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgressCategory)
event = lldb.SBEvent()
progress.Finalize()
self.assertTrue(listener.WaitForEvent(5, event))
stream = lldb.SBStream()
event.GetDescription(stream)
self.assertIn("type = end", stream.GetData())

def test_progress_finalize_deterministic_progress(self):
"""Test SBProgress finalize sends the progressEnd event"""

progress = lldb.SBProgress("Test SBProgress", "Test finalize", 13, self.dbg)
listener = lldb.SBListener("Test listener")
broadcaster = self.dbg.GetBroadcaster()
broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgressCategory)
event = lldb.SBEvent()
progress.Finalize()
self.assertTrue(listener.WaitForEvent(5, event))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't you have to wait for the first progress start event and also the end event here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope! Because we create the progress event before wiring up the listener we only listen for the end message.

stream = lldb.SBStream()
event.GetDescription(stream)
# Note even for progresses with a total, the total isn't
# sent in the end message.
self.assertIn("type = end", stream.GetData())