Skip to content
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
17 changes: 16 additions & 1 deletion RdlEngine/Definition/ReportParameter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -495,10 +495,16 @@ private object GetDataValueFromDisplay(object dvalue)

/// <summary>
/// The runtime value of the parameter.
/// Prefer using GetValueAsync() to avoid synchronous blocking on an async call.
/// </summary>
public object Value
{
get { return _rp.GetRuntimeValue(this._rpt); }
get
{
// HACK: async - synchronous wrapper needed for property getter;
// use GetValueAsync() when an async context is available.
return Task.Run(async () => await _rp.GetRuntimeValue(this._rpt)).GetAwaiter().GetResult();
}
Comment on lines +502 to +507
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

The Value property getter now uses Task.Run with GetAwaiter().GetResult() to synchronously call an async method. While this follows the codebase's "HACK: async" pattern, it introduces potential performance overhead and deadlock risks, especially in UI contexts or when called under a SynchronizationContext. This is particularly concerning because UserReportParameter.Value is accessed frequently in GetParameters() in a foreach loop (line 1489-1496), which could create multiple synchronous blocking calls in rapid succession.

Consider whether the GetParameters() method itself could be made async to properly await these values, or if there's a way to cache the values after they're first set in the UI to avoid repeated async calls during export operations.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied in commit 20c939b. Added a GetValueAsync() method to UserReportParameter that properly awaits GetRuntimeValue without blocking. Updated GetParameters() to be async Task<IDictionary> and use await urp.GetValueAsync() in the loop, eliminating the synchronous Task.Run().GetAwaiter().GetResult() pattern from the hot path. Both callers (SaveAs and GetPages) now await GetParameters().

set
{
if (this.MultiValue && value is string)
Expand All @@ -513,6 +519,15 @@ public object Value
}
}

/// <summary>
/// Asynchronously gets the runtime value of the parameter.
/// Prefer this over the synchronous Value getter when an async context is available.
/// </summary>
public async Task<object> GetValueAsync()
{
return await _rp.GetRuntimeValue(this._rpt);
}

/// <summary>
/// Take a string and parse it into multiple values
/// </summary>
Expand Down
31 changes: 28 additions & 3 deletions RdlViewer/RdlViewer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,7 @@ public async Task SaveAs(string FileName, Majorsilence.Reporting.Rdl.OutputPrese
if (!(type == OutputPresentationType.PDF || type == OutputPresentationType.PDFOldStyle ||
type == OutputPresentationType.TIF || type == OutputPresentationType.TIFBW))
{
var ld = GetParameters(); // split parms into dictionary
var ld = await GetParameters(); // split parms into dictionary
await _Report.RunGetData(ld); // obtain the data (again)
}
try
Expand Down Expand Up @@ -1445,7 +1445,7 @@ private async Task<Pages> GetPages(Report report)
{
Pages pgs = null;

var ld = GetParameters(); // split parms into dictionary
var ld = await GetParameters(); // split parms into dictionary

try
{
Expand Down Expand Up @@ -1479,8 +1479,33 @@ private async Task<Pages> GetPages(Report report)
return pgs;
}

private IDictionary GetParameters()
private async Task<IDictionary> GetParameters()
{
// If we have a loaded report with user parameters, use those runtime values
// instead of the _Parameters dictionary which may be stale
if (_Report != null && _Report.UserReportParameters != null && _Report.UserReportParameters.Count > 0)
{
Dictionary<string, object> runtimeParams = new Dictionary<string, object>();
foreach (UserReportParameter urp in _Report.UserReportParameters)
{
// Always include user parameters, even when the value is null, so that
// stale values from _Parameters cannot override user-visible parameters.
runtimeParams[urp.Name] = await urp.GetValueAsync();
}
// Merge with any parameters from _Parameters that aren't in UserReportParameters
if (_Parameters != null)
{
foreach (DictionaryEntry kvp in _Parameters)
{
string key = kvp.Key.ToString();
if (!runtimeParams.ContainsKey(key))
{
runtimeParams[key] = kvp.Value;
}
}
}
return runtimeParams;
}
return _Parameters;
}

Expand Down
Loading