Skip to content

Commit cb02cb0

Browse files
AtryAtry
committed
Improve Condition:
1. Introduce the ADT type ConditionState to replace ?Awaitable<T>, suppressing HH_FIXME[4110] 2. Added a Finish state to remove the reference from the child awaitable to the Condition, in case of memory leak 3. Added wait_for_notification_async and ConditionNotifyee to hide the setState function
1 parent d643a73 commit cb02cb0

File tree

2 files changed

+209
-53
lines changed

2 files changed

+209
-53
lines changed

‎src/_Private/ConditionState.php‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
<?hh
2+
/*
3+
* Copyright (c) 2004-present, Facebook, Inc.
4+
* All rights reserved.
5+
*
6+
* This source code is licensed under the MIT license found in the
7+
* LICENSE file in the hphp/hsl/ subdirectory of this source tree.
8+
*
9+
*/
10+
11+
namespace HH\Lib\_Private;
12+
13+
use type HH\Lib\Async\Condition;
14+
15+
<<__Sealed(
16+
NotStarted::class,
17+
SyncResult::class,
18+
AsyncResult::class,
19+
Finished::class,
20+
)>>
21+
interface ConditionState<T> {
22+
public function waitForNotificationAsync(
23+
Condition<T> $condition,
24+
Awaitable<void> $notifiers,
25+
): Awaitable<T>;
26+
public function trySucceed(Condition<T> $condition, T $result): bool;
27+
public function tryFail(Condition<T> $condition, \Exception $exception): bool;
28+
}
29+
30+
final class NotStarted<T> implements ConditionState<T> {
31+
private function __construct() {}
32+
<<__Memoize>>
33+
public static function getInstance(): this {
34+
return new self();
35+
}
36+
37+
public async function waitForNotificationAsync(
38+
Condition<T> $condition,
39+
Awaitable<void> $notifiers,
40+
): Awaitable<T> {
41+
$handle = ConditionWaitHandle::create($notifiers);
42+
$condition->setState(new AsyncResult($handle));
43+
try {
44+
return await $handle;
45+
} finally {
46+
$condition->setState(Finished::getInstance());
47+
}
48+
}
49+
50+
public function trySucceed(Condition<T> $condition, T $result): bool {
51+
$condition->setState(
52+
new SyncResult(
53+
async {
54+
return $result;
55+
},
56+
),
57+
);
58+
return true;
59+
}
60+
public function tryFail(
61+
Condition<T> $condition,
62+
\Exception $exception,
63+
): bool {
64+
$condition->setState(
65+
new SyncResult(
66+
async {
67+
throw $exception;
68+
},
69+
),
70+
);
71+
return true;
72+
}
73+
}
74+
75+
final class AsyncResult<T> implements ConditionState<T> {
76+
public function __construct(private ConditionWaitHandle<T> $resultHandle) {}
77+
public function waitForNotificationAsync(
78+
Condition<T> $_state_ref,
79+
Awaitable<void> $_notifiers,
80+
): Awaitable<T> {
81+
invariant_violation('Unable to wait for notification twice');
82+
}
83+
public function trySucceed(Condition<T> $condition, T $result): bool {
84+
$this->resultHandle->succeed($result);
85+
return true;
86+
}
87+
public function tryFail(
88+
Condition<T> $condition,
89+
\Exception $exception,
90+
): bool {
91+
$this->resultHandle->fail($exception);
92+
return true;
93+
}
94+
95+
}
96+
97+
final class SyncResult<T> implements ConditionState<T> {
98+
public function __construct(private Awaitable<T> $resultAwaitable) {}
99+
public function waitForNotificationAsync(
100+
Condition<T> $condition,
101+
Awaitable<void> $_notifiers,
102+
): Awaitable<T> {
103+
$condition->setState(Finished::getInstance());
104+
return $this->resultAwaitable;
105+
}
106+
107+
public function trySucceed(Condition<T> $condition, T $result): bool {
108+
return false;
109+
}
110+
public function tryFail(
111+
Condition<T> $condition,
112+
\Exception $exception,
113+
): bool {
114+
return false;
115+
}
116+
}
117+
118+
final class Finished<T> implements ConditionState<T> {
119+
private function __construct() {}
120+
<<__Memoize>>
121+
public static function getInstance(): this {
122+
return new self();
123+
}
124+
public function waitForNotificationAsync(
125+
Condition<T> $_state_ref,
126+
Awaitable<void> $_notifiers,
127+
): Awaitable<T> {
128+
invariant_violation('Unable to wait for notification twice');
129+
}
130+
public function trySucceed(Condition<T> $condition, T $result): bool {
131+
return false;
132+
}
133+
public function tryFail(
134+
Condition<T> $condition,
135+
\Exception $exception,
136+
): bool {
137+
return false;
138+
}
139+
}

‎src/async/Condition.php‎

Lines changed: 70 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -10,70 +10,42 @@
1010

1111
namespace HH\Lib\Async;
1212

13+
use namespace HH\Lib\_Private;
14+
1315
/**
1416
* A wrapper around ConditionWaitHandle that allows notification events
1517
* to occur before the condition is awaited.
1618
*/
17-
class Condition<T> {
18-
private ?Awaitable<T> $condition = null;
19+
class Condition<T> implements ConditionNotifyee<T> {
20+
private _Private\ConditionState<T> $state;
21+
public function __construct() {
22+
$this->state = _Private\NotStarted::getInstance();
23+
}
24+
25+
public function setState(_Private\ConditionState<T> $state): void {
26+
$this->state = $state;
27+
}
1928

20-
/**
21-
* Notify the condition variable of success and set the result.
22-
*/
2329
final public function succeed(T $result): void {
24-
invariant($this->trySucceed($result), 'Unable to notify Condition twice');
30+
invariant(
31+
$this->state->trySucceed($this, $result),
32+
'Unable to notify Condition twice',
33+
);
2534
}
2635

27-
/**
28-
* Notify the condition variable of failure and set the exception.
29-
*/
3036
final public function fail(\Exception $exception): void {
31-
invariant($this->tryFail($exception), 'Unable to notify Condition twice');
37+
invariant(
38+
$this->state->tryFail($this, $exception),
39+
'Unable to notify Condition twice',
40+
);
3241
}
3342

34-
/**
35-
* Notify the condition variable of success and set the $result.
36-
*
37-
* @return
38-
* true if the condition is set to $result successfully, false if the
39-
* condition was previously set to another result or exception.
40-
*/
4143
final public function trySucceed(T $result): bool {
42-
if ($this->condition === null) {
43-
$this->condition = async {
44-
return $result;
45-
};
46-
return true;
47-
} else {
48-
if (!($this->condition is ConditionWaitHandle<_>)) {
49-
return false;
50-
}
51-
/* HH_FIXME[4110]: Type error revealed by type-safe instanceof feature. See https://fburl.com/instanceof */
52-
$this->condition->succeed($result);
53-
return true;
54-
}
44+
return $this->state->trySucceed($this, $result);
5545
}
5646

57-
/**
58-
* Notify the condition variable of failure and set the $exception.
59-
*
60-
* @return
61-
* true if the condition is set to $exception successfully, false if the
62-
* condition was previously set to another result or exception.
63-
*/
6447
final public function tryFail(\Exception $exception): bool {
65-
if ($this->condition === null) {
66-
$this->condition = async {
67-
throw $exception;
68-
};
69-
return true;
70-
} else {
71-
if (!($this->condition is ConditionWaitHandle<_>)) {
72-
return false;
73-
}
74-
$this->condition->fail($exception);
75-
return true;
76-
}
48+
return $this->state->tryFail($this, $exception);
7749
}
7850

7951
/**
@@ -90,9 +62,54 @@ final public function tryFail(\Exception $exception): bool {
9062
final public async function waitForNotificationAsync(
9163
Awaitable<void> $notifiers,
9264
): Awaitable<T> {
93-
if ($this->condition === null) {
94-
$this->condition = ConditionWaitHandle::create($notifiers);
95-
}
96-
return await $this->condition;
65+
return await $this->state->waitForNotificationAsync($this, $notifiers);
9766
}
9867
}
68+
69+
70+
/**
71+
* Asynchronously wait for the condition variable to be notified and
72+
* return the result or throw the exception received via notification.
73+
*
74+
* The caller must provide an Awaitable $notifiers (which must be a
75+
* WaitHandle) that must not finish before the notification is received.
76+
* This means $notifiers must represent work that is guaranteed to
77+
* eventually trigger the notification. As long as the notification is
78+
* issued only once, asynchronous execution unrelated to $notifiers is
79+
* allowed to trigger the notification.
80+
*/
81+
function wait_for_notification_async<T>(
82+
(function(ConditionNotifyee<T>): Awaitable<void>) $notifiers,
83+
): Awaitable<T> {
84+
$condition = new Condition();
85+
return $condition->waitForNotificationAsync($notifiers($condition));
86+
}
87+
88+
interface ConditionNotifyee<-T> {
89+
90+
/**
91+
* Notify the condition variable of success and set the $result.
92+
*/
93+
public function succeed(T $result): void;
94+
/**
95+
* Notify the condition variable of success and set the $result.
96+
*
97+
* @return
98+
* true if the condition is set to $result successfully, false if the
99+
* condition was previously set to another result or exception.
100+
*/
101+
public function trySucceed(T $result): bool;
102+
103+
/**
104+
* Notify the condition variable of failure and set the exception.
105+
*/
106+
public function fail(\Exception $exception): void;
107+
/**
108+
* Notify the condition variable of failure and set the $exception.
109+
*
110+
* @return
111+
* true if the condition is set to $exception successfully, false if the
112+
* condition was previously set to another result or exception.
113+
*/
114+
public function tryFail(\Exception $exception): bool;
115+
}

0 commit comments

Comments
 (0)