Paths

Table of Contentst

Differential D58385

pwait: Test the new -r option
ClosedPublic
Actions

Authored by des on Tue, Jul 21, 8:08 PM.

Tags

None

Referenced Files

	F164879286: D58385.diff
	Tue, Aug 4, 11:07 AM

	F164809805: D58385.id.diff
	Tue, Aug 4, 3:57 AM

	F164800346: D58385.id182621.diff
	Tue, Aug 4, 2:54 AM

	F164792870: D58385.id182771.diff
	Tue, Aug 4, 2:07 AM

	F164788585: D58385.id182418.diff
	Tue, Aug 4, 1:48 AM

	F164785193: D58385.id182418.diff
	Tue, Aug 4, 1:05 AM

	F164782964: D58385.id182771.diff
	Tue, Aug 4, 12:39 AM

	Unknown Object (File)
	Mon, Aug 3, 10:50 PM

View All 49 Files

Subscribers

Details

Reviewers

kib
kevans
markj

Group Reviewers

Commits

rG4e6679c9c422: pwait: Test the new -r option
rGe87afada8e8a: pwait: Test the new -r option
rGe115066370dc: pwait: Test the new -r option

Summary

Test that pwait without -r reports a process as soon as it terminates,
while pwait with -r does not report it until it has been reaped.

MFC after: 1 week
Sponsored by: Klara, Inc.
Sponsored by: NetApp, Inc.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 74989
Build 71872: arc lint + arc unit

Event Timeline

des created this revision.Tue, Jul 21, 8:08 PM

Herald added a subscriber: imp. · View Herald TranscriptTue, Jul 21, 8:08 PM

des requested review of this revision.Tue, Jul 21, 8:08 PM

Harbormaster completed remote builds in B74989: Diff 182418.Tue, Jul 21, 8:08 PM

des added a parent revision: D58314: pwait: Optionally wait until process is reaped.Tue, Jul 21, 8:09 PM

des added a parent revision: D58386: pwait: Add a SIGINFO handler.Tue, Jul 21, 8:19 PM

des retitled this revision from pwait: Test that pwait does not report zombies to pwait: Test the new -r option.Wed, Jul 22, 8:16 AM

des edited the summary of this revision. (Show Details)

new behavior is now optional, test both variants

Harbormaster completed remote builds in B74997: Diff 182439.Wed, Jul 22, 8:18 AM

rename test

Harbormaster completed remote builds in B75001: Diff 182443.Wed, Jul 22, 8:33 AM

kib added inline comments.Wed, Jul 22, 11:44 PM

bin/pwait/tests/pwait_reap.c
125 ↗	(On Diff #182443)	Why not using posix_spawn() there?
135 ↗	(On Diff #182443)	You are not waiting for the termination there, wait_zero(false) only checks for termination and asserts if not. As the consequence, the test is racy, since nothing guarantees that the target has exited when waitpid() is called.

des added inline comments.Thu, Jul 23, 6:20 AM

bin/pwait/tests/pwait_reap.c
125 ↗	(On Diff #182443)	What would be the advantage?
135 ↗	(On Diff #182443)	I think you're confusing `WNOWAIT` with `WNOHANG`.

des added a reviewer: Klara.Thu, Jul 23, 5:50 PM

kib added inline comments.Thu, Jul 23, 11:32 PM

bin/pwait/tests/pwait_reap.c
125 ↗	(On Diff #182443)	That the used API is specifically designed for the task at hand.
135 ↗	(On Diff #182443)	No, I do not. Re-read what I wrote: wait_zero(false) may assert falsely.

des added inline comments.Fri, Jul 24, 10:00 AM

bin/pwait/tests/pwait_reap.c
135 ↗	(On Diff #182443)	No. It will block until the target terminates. It will fail if interrupted, but I don't consider that a problem.

tolerate EINTR

Harbormaster completed remote builds in B75098: Diff 182621.Fri, Jul 24, 4:28 PM

kib accepted this revision.Fri, Jul 24, 10:02 PM

This revision is now accepted and ready to land.Fri, Jul 24, 10:02 PM

Closed by commit rGe115066370dc: pwait: Test the new -r option (authored by des). · Explain WhyMon, Jul 27, 10:17 AM

This revision was automatically updated to reflect the committed changes.

des added a commit: rGe115066370dc: pwait: Test the new -r option.

markj added inline comments.Mon, Jul 27, 1:13 PM

bin/pwait/tests/pwait_reap.c
169 ↗	(On Diff #182771)	Shouldn't this be `pwait_reap_test(false)`?

des added inline comments.Mon, Jul 27, 6:49 PM

bin/pwait/tests/pwait_reap.c
169 ↗	(On Diff #182771)	You're right, will fix.

des added a commit: rGe87afada8e8a: pwait: Test the new -r option.Mon, Aug 3, 11:39 AM

des added a commit: rG4e6679c9c422: pwait: Test the new -r option.

Revision Contents
Changeset List

Path

Size

bin/

pwait/

tests/

1 line

122 lines

Diff 182418

bin/pwait/tests/Makefile

Loading...

bin/pwait/tests/pwait_zombie.c

Loading...