Skip to content
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

Improve manager #156

Merged
merged 6 commits into from
Nov 28, 2024
Merged

Improve manager #156

merged 6 commits into from
Nov 28, 2024

Conversation

kazu-yamamoto
Copy link
Owner

@kazu-yamamoto kazu-yamamoto commented Nov 26, 2024

- use "bracket"
- ignore "KilledByHttp2ThreadManager" only as "KilledByHttp2ThreadManager" is thrown here
ensuring that only one asynchronous exception is thrown
@kazu-yamamoto kazu-yamamoto requested a review from edsko November 26, 2024 03:04
@kazu-yamamoto
Copy link
Owner Author

I confirmed that modifyTVar' uses $!.

modifyTVar' var f = do
    x <- readTVar var
    writeTVar var $! f x

@kazu-yamamoto
Copy link
Owner Author

I'm going to merge this PR.

@kazu-yamamoto kazu-yamamoto merged commit b054b01 into main Nov 28, 2024
10 checks passed
@kazu-yamamoto kazu-yamamoto deleted the improve-manager branch November 28, 2024 01:05
myWeakThradId = do
tid <- myThreadId
wtid <- mkWeakThreadId tid
let n = read (drop 9 $ show tid) -- drop "ThreadId "

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kazu-yamamoto I am late to the party but there’s fromThreadId in base. It allows you to get a stable integer identifier of a thread without resorting to the show instance.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find fromThreadId anywhere?
Which module exports it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fromThreadId is very new, sigh.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I don't think the weak references are strictly necessary. Provided that we keep the Map in WHNF, and we remove entries whenever the corresponding threads die, we should not be unnecessarily retaining any threads.

@kazu-yamamoto
Copy link
Owner Author

You are right in the current code.
Weak ThreadId is a kind of over-spec at this moment.

The old System.TimeManager did not delete an entry when its thread dies...

@edsko edsko mentioned this pull request Dec 4, 2024
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 30, 2025
# ChangeLog for http2

## 5.3.9

* Using `ThreadManager` of `time-manager`.

## 5.3.8

* `forkManagedTimeout` ensures that only one asynchronous exception is
  thrown. Fixing the thread leak via `Weak ThreadId` and `modifyTVar'`.
  [#156](kazu-yamamoto/http2#156)

## 5.3.7

* Using `withHandle` of time-manager.
* Getting `Handle` for each thread.
* Providing allocSimpleConfig' to enable customizing WAI tiemout manager.
* Monitor option (-m) for h2c-client and h2c-server.

## 5.3.6

* Making `runIO` friendly with the new synchronism mechanism.
  [#152](kazu-yamamoto/http2#152)
* Re-throwing asynchronous exceptions to prevent thread leak.
* Simplifying the synchronism mechanism between workers and the sender.
  [#148](kazu-yamamoto/http2#148)

## 5.3.5

* Using `http-semantics` v0.3.
* Deprecating `numberOfWorkers`.
* Removing `unliftio`.
* Avoid `undefined` in client.
  [#146](kazu-yamamoto/http2#146)

## 5.3.4

* Support stream cancellation
  [#142](kazu-yamamoto/http2#142)

## 5.3.3

* Enclosing IPv6 literal authority with square brackets.
  [#143](kazu-yamamoto/http2#143)

## 5.3.2

* Avoid unnecessary empty data frames at end of stream
  [#140](kazu-yamamoto/http2#140)
* Removing unnecessary API from ServerIO

## 5.3.1

* Fix treatment of async exceptions
  [#138](kazu-yamamoto/http2#138)
* Avoid race condition
  [#137](kazu-yamamoto/http2#137)

## 5.3.0

* New server architecture: spawning worker on demand instead of the
  worker pool. This reduce huge numbers of threads for streaming into
  only 2. No API changes but workers do not terminate quicly. Rather
  workers collaborate with the sender after queuing a response and
  finish after all response data are sent.
* All threads are labeled with `labelThread`. You can see them by
  `listThreas` if necessary.

## 5.2.6

* Recover rxflow on closing.
  [#126](kazu-yamamoto/http2#126)
* Fixing ClientSpec for stream errors.
* Allowing negative window. (h2spec http2/6.9.2)
* Update for latest http-semantics
  [#122](kazu-yamamoto/http2#124)

## 5.2.5

* Setting peer initial window size properly.
  [#123](kazu-yamamoto/http2#123)

## 5.2.4

* Update for latest http-semantics
  [#122](kazu-yamamoto/http2#122)
* Measuring performance concurrently for h2c-client

## 5.2.3

* Update for latest http-semantics
  [#120](kazu-yamamoto/http2#120)
* Enable containers 0.7 (ghc 9.10)
  [#117](kazu-yamamoto/http2#117)

## 5.2.2

* Mark final chunk as final
  [#116](kazu-yamamoto/http2#116)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants