Add a guard check against stack overflow when initializing BuildData#486
Conversation
nikita-tkachenko-datadog
added
the
changelog/Fixed
| if (!BUILD_DATA_BEING_CREATED.get().add(run)) { | ||
| String runName = run != null ? run.getDisplayName() : null; | ||
| DatadogUtilities.severe(LOGGER, null, "BuildData creation is in progress for run " + runName + "; using empty data"); | ||
| return BuildData.EMPTY; |
There was a problem hiding this comment.
Although we return EMPTY here, we know that there's a call in the stack that will return the populated data? Is my assumption correct?
There was a problem hiding this comment.
Yes, exactly.
In short, the problem is that constructing a BuildData instance may trigger some run state to be initialised, and this results in Jenkins printing some logs to the console (something like "reading state for run #N from disk").
The plugin wants to send these logs to Datadog, just like any other logs. And it wants to tag each of these logs with the data of the associated run. But that requires a BuildData, so we enter an endless cycle.
By returning EMPTY we break this cycle. The downside is that logs triggered by BuildData init will miss the run info (they will only be tagged with the common Jenkins instance info).
|
Thanks for fix @nikita-tkachenko-datadog |
Hi @edwinmlobo, plugin v9.0.0 with the fix has just been released. |
3 participants
Requirements for Contributing to this repository
What does this PR do?
Adds a workaround for a
BuildDatainitialization issue.Some of the fields in this
BuildDataare initialized with the values obtained from aRuninstance.Querying
Runfields can trigger run initialization in some cases (e.g. loading run data from disk when resuming a run after Jenkins restart).During run initialization some logs are written.
Writing logs, in turn, requires creating a
TaskListenerassociated with the run.Creating the listener triggers initialization of
DatadogTaskListenerDecorator, as the listener's logger needs to be decorated.The decorator creates another
BuildDatainstance, whose creation starts the whole cycle from the beginning.As the result, the code enters an endless cycle of initializing
BuildDatawhile initializingBuildData, which terminates with a stack overflow.The added code checks that
BuildDatafor the provided run is already being created in the current thread.If that is the case, empty
BuildDatainstance is returned for the nested calls in order to break the cycle.As a side effect, whatever logs are written while Run instance is being initialized will not be tagged with that run's data.
It should fix the following issues:
#484
#389
Description of the Change
Alternate Designs
Possible Drawbacks
Verification Process
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/label attached. If applicable it should have thebackward-incompatiblelabel attached.do-not-merge/label attached.kind/andseverity/labels attached at least.