aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-04-29 14:28:57 +0100
committerGitHub <[email protected]>2020-04-29 14:28:57 +0100
commit1cde354c35f425026184b8d72f4f5865e96975d4 (patch)
tree11fa53f91ff2bf598f8ba726d130708bf7b3bad2
parent12aae7771dc220a62d1323ac6a30ddf215fe2b92 (diff)
parentbfce6573772ebb91a9b1054864c0f53669ceee2f (diff)
Merge #4119
4119: Cache proc-macro dlls r=matklad a=edwin0cheng This PR try to fix a deadlock in proc-macro srv by not unloading dlls. Currently we load and unload dlls for each request, however rustc TLS is leaky , such that if we do it a lot of times, all TLS index will be consumed and it will be deadlocked inside panic (it is because panic itself is using TLS too). Co-authored-by: Edwin Cheng <[email protected]>
-rw-r--r--crates/ra_proc_macro_srv/src/cli.rs8
-rw-r--r--crates/ra_proc_macro_srv/src/dylib.rs113
-rw-r--r--crates/ra_proc_macro_srv/src/lib.rs48
-rw-r--r--crates/ra_proc_macro_srv/src/tests/utils.rs6
4 files changed, 110 insertions, 65 deletions
diff --git a/crates/ra_proc_macro_srv/src/cli.rs b/crates/ra_proc_macro_srv/src/cli.rs
index 7282e5b9b..1437794c9 100644
--- a/crates/ra_proc_macro_srv/src/cli.rs
+++ b/crates/ra_proc_macro_srv/src/cli.rs
@@ -1,15 +1,17 @@
1//! Driver for proc macro server 1//! Driver for proc macro server
2 2
3use crate::{expand_task, list_macros}; 3use crate::ProcMacroSrv;
4use ra_proc_macro::msg::{self, Message}; 4use ra_proc_macro::msg::{self, Message};
5use std::io; 5use std::io;
6 6
7pub fn run() -> io::Result<()> { 7pub fn run() -> io::Result<()> {
8 let mut srv = ProcMacroSrv::default();
9
8 while let Some(req) = read_request()? { 10 while let Some(req) = read_request()? {
9 let res = match req { 11 let res = match req {
10 msg::Request::ListMacro(task) => Ok(msg::Response::ListMacro(list_macros(&task))), 12 msg::Request::ListMacro(task) => srv.list_macros(&task).map(msg::Response::ListMacro),
11 msg::Request::ExpansionMacro(task) => { 13 msg::Request::ExpansionMacro(task) => {
12 expand_task(&task).map(msg::Response::ExpansionMacro) 14 srv.expand(&task).map(msg::Response::ExpansionMacro)
13 } 15 }
14 }; 16 };
15 17
diff --git a/crates/ra_proc_macro_srv/src/dylib.rs b/crates/ra_proc_macro_srv/src/dylib.rs
index d202eb0fd..aa84e951c 100644
--- a/crates/ra_proc_macro_srv/src/dylib.rs
+++ b/crates/ra_proc_macro_srv/src/dylib.rs
@@ -2,13 +2,12 @@
2 2
3use crate::{proc_macro::bridge, rustc_server::TokenStream}; 3use crate::{proc_macro::bridge, rustc_server::TokenStream};
4use std::fs::File; 4use std::fs::File;
5use std::path::Path; 5use std::path::{Path, PathBuf};
6 6
7use goblin::{mach::Mach, Object}; 7use goblin::{mach::Mach, Object};
8use libloading::Library; 8use libloading::Library;
9use memmap::Mmap; 9use memmap::Mmap;
10use ra_proc_macro::ProcMacroKind; 10use ra_proc_macro::ProcMacroKind;
11
12use std::io; 11use std::io;
13 12
14const NEW_REGISTRAR_SYMBOL: &str = "_rustc_proc_macro_decls_"; 13const NEW_REGISTRAR_SYMBOL: &str = "_rustc_proc_macro_decls_";
@@ -109,23 +108,21 @@ impl ProcMacroLibraryLibloading {
109 } 108 }
110} 109}
111 110
112type ProcMacroLibraryImpl = ProcMacroLibraryLibloading;
113
114pub struct Expander { 111pub struct Expander {
115 libs: Vec<ProcMacroLibraryImpl>, 112 inner: ProcMacroLibraryLibloading,
116} 113}
117 114
118impl Expander { 115impl Expander {
119 pub fn new(lib: &Path) -> Result<Expander, String> { 116 pub fn new(lib: &Path) -> io::Result<Expander> {
120 // Some libraries for dynamic loading require canonicalized path even when it is 117 // Some libraries for dynamic loading require canonicalized path even when it is
121 // already absolute 118 // already absolute
122 let lib = lib 119 let lib = lib.canonicalize()?;
123 .canonicalize() 120
124 .unwrap_or_else(|err| panic!("Cannot canonicalize {}: {:?}", lib.display(), err)); 121 let lib = ensure_file_with_lock_free_access(&lib)?;
125 122
126 let library = ProcMacroLibraryImpl::open(&lib).map_err(|e| e.to_string())?; 123 let library = ProcMacroLibraryLibloading::open(&lib)?;
127 124
128 Ok(Expander { libs: vec![library] }) 125 Ok(Expander { inner: library })
129 } 126 }
130 127
131 pub fn expand( 128 pub fn expand(
@@ -141,38 +138,36 @@ impl Expander {
141 TokenStream::with_subtree(attr.clone()) 138 TokenStream::with_subtree(attr.clone())
142 }); 139 });
143 140
144 for lib in &self.libs { 141 for proc_macro in &self.inner.exported_macros {
145 for proc_macro in &lib.exported_macros { 142 match proc_macro {
146 match proc_macro { 143 bridge::client::ProcMacro::CustomDerive { trait_name, client, .. }
147 bridge::client::ProcMacro::CustomDerive { trait_name, client, .. } 144 if *trait_name == macro_name =>
148 if *trait_name == macro_name => 145 {
149 { 146 let res = client.run(
150 let res = client.run( 147 &crate::proc_macro::bridge::server::SameThread,
151 &crate::proc_macro::bridge::server::SameThread, 148 crate::rustc_server::Rustc::default(),
152 crate::rustc_server::Rustc::default(), 149 parsed_body,
153 parsed_body, 150 );
154 ); 151 return res.map(|it| it.subtree);
155 return res.map(|it| it.subtree); 152 }
156 } 153 bridge::client::ProcMacro::Bang { name, client } if *name == macro_name => {
157 bridge::client::ProcMacro::Bang { name, client } if *name == macro_name => { 154 let res = client.run(
158 let res = client.run( 155 &crate::proc_macro::bridge::server::SameThread,
159 &crate::proc_macro::bridge::server::SameThread, 156 crate::rustc_server::Rustc::default(),
160 crate::rustc_server::Rustc::default(), 157 parsed_body,
161 parsed_body, 158 );
162 ); 159 return res.map(|it| it.subtree);
163 return res.map(|it| it.subtree); 160 }
164 } 161 bridge::client::ProcMacro::Attr { name, client } if *name == macro_name => {
165 bridge::client::ProcMacro::Attr { name, client } if *name == macro_name => { 162 let res = client.run(
166 let res = client.run( 163 &crate::proc_macro::bridge::server::SameThread,
167 &crate::proc_macro::bridge::server::SameThread, 164 crate::rustc_server::Rustc::default(),
168 crate::rustc_server::Rustc::default(), 165 parsed_attributes,
169 parsed_attributes, 166 parsed_body,
170 parsed_body, 167 );
171 ); 168 return res.map(|it| it.subtree);
172 return res.map(|it| it.subtree);
173 }
174 _ => continue,
175 } 169 }
170 _ => continue,
176 } 171 }
177 } 172 }
178 173
@@ -180,9 +175,9 @@ impl Expander {
180 } 175 }
181 176
182 pub fn list_macros(&self) -> Vec<(String, ProcMacroKind)> { 177 pub fn list_macros(&self) -> Vec<(String, ProcMacroKind)> {
183 self.libs 178 self.inner
179 .exported_macros
184 .iter() 180 .iter()
185 .flat_map(|it| &it.exported_macros)
186 .map(|proc_macro| match proc_macro { 181 .map(|proc_macro| match proc_macro {
187 bridge::client::ProcMacro::CustomDerive { trait_name, .. } => { 182 bridge::client::ProcMacro::CustomDerive { trait_name, .. } => {
188 (trait_name.to_string(), ProcMacroKind::CustomDerive) 183 (trait_name.to_string(), ProcMacroKind::CustomDerive)
@@ -197,3 +192,33 @@ impl Expander {
197 .collect() 192 .collect()
198 } 193 }
199} 194}
195
196/// Copy the dylib to temp directory to prevent locking in Windows
197#[cfg(windows)]
198fn ensure_file_with_lock_free_access(path: &Path) -> io::Result<PathBuf> {
199 use std::{ffi::OsString, time::SystemTime};
200
201 let mut to = std::env::temp_dir();
202
203 let file_name = path.file_name().ok_or_else(|| {
204 io::Error::new(
205 io::ErrorKind::InvalidInput,
206 format!("File path is invalid: {}", path.display()),
207 )
208 })?;
209
210 // generate a time deps unique number
211 let t = SystemTime::now().duration_since(std::time::UNIX_EPOCH).expect("Time went backwards");
212
213 let mut unique_name = OsString::from(t.as_millis().to_string());
214 unique_name.push(file_name);
215
216 to.push(unique_name);
217 std::fs::copy(path, &to).unwrap();
218 Ok(to)
219}
220
221#[cfg(unix)]
222fn ensure_file_with_lock_free_access(path: &Path) -> io::Result<PathBuf> {
223 Ok(path.to_path_buf())
224}
diff --git a/crates/ra_proc_macro_srv/src/lib.rs b/crates/ra_proc_macro_srv/src/lib.rs
index 3aca859db..922bb84bb 100644
--- a/crates/ra_proc_macro_srv/src/lib.rs
+++ b/crates/ra_proc_macro_srv/src/lib.rs
@@ -21,28 +21,46 @@ mod dylib;
21 21
22use proc_macro::bridge::client::TokenStream; 22use proc_macro::bridge::client::TokenStream;
23use ra_proc_macro::{ExpansionResult, ExpansionTask, ListMacrosResult, ListMacrosTask}; 23use ra_proc_macro::{ExpansionResult, ExpansionTask, ListMacrosResult, ListMacrosTask};
24use std::path::Path; 24use std::{
25 collections::{hash_map::Entry, HashMap},
26 fs,
27 path::{Path, PathBuf},
28 time::SystemTime,
29};
25 30
26pub(crate) fn expand_task(task: &ExpansionTask) -> Result<ExpansionResult, String> { 31#[derive(Default)]
27 let expander = create_expander(&task.lib); 32pub(crate) struct ProcMacroSrv {
33 expanders: HashMap<(PathBuf, SystemTime), dylib::Expander>,
34}
28 35
29 match expander.expand(&task.macro_name, &task.macro_body, task.attributes.as_ref()) { 36impl ProcMacroSrv {
30 Ok(expansion) => Ok(ExpansionResult { expansion }), 37 pub fn expand(&mut self, task: &ExpansionTask) -> Result<ExpansionResult, String> {
31 Err(msg) => { 38 let expander = self.expander(&task.lib)?;
32 Err(format!("Cannot perform expansion for {}: error {:?}", &task.macro_name, msg)) 39 match expander.expand(&task.macro_name, &task.macro_body, task.attributes.as_ref()) {
40 Ok(expansion) => Ok(ExpansionResult { expansion }),
41 Err(msg) => {
42 Err(format!("Cannot perform expansion for {}: error {:?}", &task.macro_name, msg))
43 }
33 } 44 }
34 } 45 }
35}
36 46
37pub(crate) fn list_macros(task: &ListMacrosTask) -> ListMacrosResult { 47 pub fn list_macros(&mut self, task: &ListMacrosTask) -> Result<ListMacrosResult, String> {
38 let expander = create_expander(&task.lib); 48 let expander = self.expander(&task.lib)?;
49 Ok(ListMacrosResult { macros: expander.list_macros() })
50 }
39 51
40 ListMacrosResult { macros: expander.list_macros() } 52 fn expander(&mut self, path: &Path) -> Result<&dylib::Expander, String> {
41} 53 let time = fs::metadata(path).and_then(|it| it.modified()).map_err(|err| {
54 format!("Failed to get file metadata for {}: {:?}", path.display(), err)
55 })?;
42 56
43fn create_expander(lib: &Path) -> dylib::Expander { 57 Ok(match self.expanders.entry((path.to_path_buf(), time)) {
44 dylib::Expander::new(lib) 58 Entry::Vacant(v) => v.insert(dylib::Expander::new(path).map_err(|err| {
45 .unwrap_or_else(|err| panic!("Cannot create expander for {}: {:?}", lib.display(), err)) 59 format!("Cannot create expander for {}: {:?}", path.display(), err)
60 })?),
61 Entry::Occupied(e) => e.into_mut(),
62 })
63 }
46} 64}
47 65
48pub mod cli; 66pub mod cli;
diff --git a/crates/ra_proc_macro_srv/src/tests/utils.rs b/crates/ra_proc_macro_srv/src/tests/utils.rs
index 2139ec7a4..646a427c5 100644
--- a/crates/ra_proc_macro_srv/src/tests/utils.rs
+++ b/crates/ra_proc_macro_srv/src/tests/utils.rs
@@ -1,7 +1,7 @@
1//! utils used in proc-macro tests 1//! utils used in proc-macro tests
2 2
3use crate::dylib; 3use crate::dylib;
4use crate::list_macros; 4use crate::ProcMacroSrv;
5pub use difference::Changeset as __Changeset; 5pub use difference::Changeset as __Changeset;
6use ra_proc_macro::ListMacrosTask; 6use ra_proc_macro::ListMacrosTask;
7use std::str::FromStr; 7use std::str::FromStr;
@@ -59,7 +59,7 @@ pub fn assert_expand(
59pub fn list(crate_name: &str, version: &str) -> Vec<String> { 59pub fn list(crate_name: &str, version: &str) -> Vec<String> {
60 let path = fixtures::dylib_path(crate_name, version); 60 let path = fixtures::dylib_path(crate_name, version);
61 let task = ListMacrosTask { lib: path }; 61 let task = ListMacrosTask { lib: path };
62 62 let mut srv = ProcMacroSrv::default();
63 let res = list_macros(&task); 63 let res = srv.list_macros(&task).unwrap();
64 res.macros.into_iter().map(|(name, kind)| format!("{} [{:?}]", name, kind)).collect() 64 res.macros.into_iter().map(|(name, kind)| format!("{} [{:?}]", name, kind)).collect()
65} 65}