-
Notifications
You must be signed in to change notification settings - Fork 304
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
HPCC-32961 OwnedSpanScope assignment causes early span end #19288
Conversation
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-32961 Jirabot Action Result: |
@jpmcmu I think this is a good change, but I cannot reproduce your symptoms.
and it only output the tracing once. I will merge this change anyway, because it is cleaner, but what is the example code you were hitting a problem on? |
@ghalliday I was expecting I will remove the operator=(ISpan*) in favor of the move assignment operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpmcmu see comments
system/jlib/jtrace.hpp
Outdated
~OwnedSpanScope(); | ||
|
||
inline ISpan * operator -> () const { return span; } | ||
inline operator ISpan *() const { return span; } | ||
|
||
inline OwnedSpanScope& operator=(ISpan * ptr) = delete; | ||
inline OwnedSpanScope& operator=(const OwnedSpanScope& rhs) = delete; | ||
inline OwnedSpanScope& operator=(OwnedSpanScope&& rhs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given thoughts on HPCC-32982 I think this should probably be deleted. I'm not sure it makes sense because of the additional semantics of setting the thread active span - the user should only use set/setown.
- Deleted operators that could potentially cause issues Signed-off-by: James McMullan [email protected]
@ghalliday Pushed up changes for deleted move assignment operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good.
@jpmcmu for the future, please try avoiding pushing a new commit, squashing and rebasing at the same time - it makes it hard to track what has changed. |
277538e
into
hpcc-systems:candidate-9.8.x
Jirabot Action Result: |
Signed-off-by: James McMullan [email protected]
Type of change:
Checklist:
Smoketest:
Testing: