-
Notifications
You must be signed in to change notification settings - Fork 503
invoke user defined join() #922
base: master
Are you sure you want to change the base?
Conversation
It appears to, yes. Haven't looked at this in close detail yet though. 👍 |
src/object.c
Outdated
} | ||
} | ||
} zend_catch { | ||
pthreads_monitor_add(object->monitor, PTHREADS_MONITOR_ERROR); |
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.
isn't this case used for when the thread itself errored, as opposed to the parent thread crashing trying to stop it?
src/object.c
Outdated
|
||
ZVAL_UNDEF(&zresult); | ||
|
||
pthreads_monitor_add(object->monitor, PTHREADS_MONITOR_RUNNING); |
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.
perhaps not what you intended?
src/object.c
Outdated
zval_ptr_dtor(&zresult); | ||
} | ||
|
||
pthreads_monitor_remove(object->monitor, PTHREADS_MONITOR_RUNNING); |
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.
Is this necessary? I was under the impression that this would already have happened by this point.
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.
sorry, had copied the whole stuff from another function to see if it's working. now it's clean.
@dktapps does that correspond to your issue #919?